Java ArrayList Remove. Synchronize?

Started by
10 comments, last by noodleBowl 8 years, 9 months ago

If there's only one lock involved then it will never deadlock. The problem happens when the code you're calling in the loop needs to acquire *other* locks. I see this a lot when people implement their own event dispatch mechanisms and hold a lock while they iterate over the listeners list (because you need to be thread-safe, of course). The deadlock potential happens whenever locks are acquired in an inconsistent order, which is easy to do by accident when you're dealing with event listeners.

Consider this scenario: There are two objects (call them Foo and Bar) that need to cooperate. Foo listens to Bar for change notification events. Both Foo and Bar are thread-safe and have their own locks. Now imagine that on one thread Bar is dispatching events. It first acquires its own lock and then starts calling listeners *inside the synchronized block*, one of which is Foo's listener. Meanwhile, on a different thread, Foo is executing a piece of code that first acquires its own lock and then deregisters its listener from Bar (perhaps it doesn't need notifications anymore). The call to Bar.removeListener() blocks because the event dispatch thread is currently holding Bar's lock. Now, the event dispatch thread tries to notify Foo's listener and the first thing it does is try to acquire Foo's lock (so that it can change Foo's state in a thread-safe way), only it can't because the other thread is already holding Foo's lock. At this point the threads have deadlocked because they are both holding a lock that the other needs and neither can make any progress.

When I write code I have a rule: NEVER hold a lock when you're making callbacks to code you don't own and can't predict. That rule does not apply to the OP because (I assume) that whatever is happening inside his loop is known in advance (and probably isn't a callback to an event listener). So in theory, it should be possible to analyze the methods that are called and verify that they either don't need to acquire any other locks or that they acquire them in a consistent order. In practice, that kind of analysis is hard to do and easy to get wrong so I try to avoid it whenever I can by simply not holding locks if I don't have to. In this case CopyOnWriteArrayList provides a simple and effective way of making the code thread-safe, which is why I recommended it.

Advertisement

If there's only one lock involved then it will never deadlock. The problem happens when the code you're calling in the loop needs to acquire *other* locks. I see this a lot when people implement their own event dispatch mechanisms and hold a lock while they iterate over the listeners list (because you need to be thread-safe, of course). The deadlock potential happens whenever locks are acquired in an inconsistent order, which is easy to do by accident when you're dealing with event listeners.

Consider this scenario: There are two objects (call them Foo and Bar) that need to cooperate. Foo listens to Bar for change notification events. Both Foo and Bar are thread-safe and have their own locks. Now imagine that on one thread Bar is dispatching events. It first acquires its own lock and then starts calling listeners *inside the synchronized block*, one of which is Foo's listener. Meanwhile, on a different thread, Foo is executing a piece of code that first acquires its own lock and then deregisters its listener from Bar (perhaps it doesn't need notifications anymore). The call to Bar.removeListener() blocks because the event dispatch thread is currently holding Bar's lock. Now, the event dispatch thread tries to notify Foo's listener and the first thing it does is try to acquire Foo's lock (so that it can change Foo's state in a thread-safe way), only it can't because the other thread is already holding Foo's lock. At this point the threads have deadlocked because they are both holding a lock that the other needs and neither can make any progress.

This explanation is actually really helpful! Thanks

When I write code I have a rule: NEVER hold a lock when you're making callbacks to code you don't own and can't predict. That rule does not apply to the OP because (I assume) that whatever is happening inside his loop is known in advance (and probably isn't a callback to an event listener). So in theory, it should be possible to analyze the methods that are called and verify that they either don't need to acquire any other locks or that they acquire them in a consistent order. In practice, that kind of analysis is hard to do and easy to get wrong so I try to avoid it whenever I can by simply not holding locks if I don't have to. In this case CopyOnWriteArrayList provides a simple and effective way of making the code thread-safe, which is why I recommended it.

I believe this is true, but I might totally wrong. I'm talking about the "isn't a callback to an event listener part" part. I mean the event listener is a call back for when a touch event is triggered (I'm working with Android).


 
public class myInput implements View.OnTouchListener
 

But I'm not putting a lock on the whole class. In the end I went with ConcurrentHashmaps which are supposed to be thread-safe. In addition I have used synchronized methods, whenever I need to manipulate the hashmaps.

This topic is closed to new replies.

Advertisement