Conccurent Modification and Synchronization of arraylists

Question

This question is more about asking if my way of doing something is the "correct" way or not. I have some program that involves constantly updating graphical components. To that effect, I have the method below.

public void update(){
    for (BaseGameEntity movingEntity : movingEntityList) {
        ((MovingEntity)movingEntity).update();
    }
}

Essentially, the class containing this method has a list of all graphical objects that need updating and it loops through, calling their respective update methods.

The issue comes when I have to add new entities or remove current entities from this list. The addition and removal of entities is handled by a different thread, and as you can guess, this results in a Concurrent Modification Exception if I try to add/remove entities while also looping through and updating their graphical components.

My ad hoc solution was to simply throw a try-catch block around this and just ignore any concurrent modification exceptions that crop up - in effect, not updating at that specific time. This does exactly what I want and no problems occur.

public void update(){
    try{
        for (BaseGameEntity movingEntity : movingEntityList) {
            ((MovingEntity)movingEntity).update();
        }
    }catch(ConcurrentModificationException e){
            //Do Nothing
    }
}

However, my question is, is this a "proper" way of handling this issue? Should I perhaps be doing something akin to what is outlined in this answer? What is the "correct" way to handle this issue, if mine is wrong? I'm not looking specifically for ways to make my arraylist thread safe such as through synchronized lists, I'm specifically asking if my method is a valid method or if there is some reason I should avoid it and actually use a synchronized list.

Answer 1

The proper way would be to synchronize the list with Collections.synchronizedList():

List list = Collections.synchronizedList(new ArrayList());
      ...
  synchronized (list) {
      Iterator i = list.iterator(); // Must be in synchronized block
      while (i.hasNext())
          foo(i.next());
  }

If you are traversing way more than the number of times you update your list, you can also use CopyOnWriteArrayList.

If you don't mind occasional missing updates (or if they happen way too infrequently for the price of synchronization), your way is fine.

Answer 2

Is this a "proper" way of handling this issue?

If you do not mind getting an increase of concurrency at the expense of dropping the updates on error, then the answer is "yes". You do run the risk of not completing an update multiple times in a row, when significant additions and removals to the list happen often.

On the other hand, when the frequency of updates is significantly higher than the frequency of adding/removing an object, this solution sounds reasonable.

Should I perhaps be [using synchronized]?

This is also a viable solution. The difference is that an update would no longer be able to proceed when an update is in progress. This may not be desirable when the timing of calls to update is critical (yet it is not critical to update everything on every single call).

Answer 3

Some people consider it as a duplicate of all the generic synchronization questions. I think this is not the case. You are asking for a very specific constellation, and whether your solution is "OK", in this sense.

Based on what you described, the actual goal seems to be clear: You want to quickly and concurrently iterate over the entities to call the update method, and avoid any synchronization overhead that may be implied by using Collections#synchronizedList or similar approaches.

Additionally, I assume that the main idea behind the solution that you proposed was that the update calls have to be done very often and as fast as possible, whereas adding or removing entities happens "rarely".

^{So, adding and removing elements is an exception, compared to the regular operations ;-)}

And (as dasblinkenlight already pointed out in his answer) for such a setup, the solution of catching and ignoring the ConcurrentModificationException is reasonable, but you should be aware of the consequences.

It might happen that the update method of some entities is called, and then the loop bails out due to the ConcurrentModificationException. You should be absolutely sure that this does not have undesirable side-effects. Depending on what update actually does, this might, for example, cause some entities to move faster over the screen, and others to not move at all, because their update calls had been missed due to several ConcurrentModificationExceptions. This may be particularly problematic if adding and removing entities is not an operation that happens rarely: If one thread constantly adds or removes elements, then the last elements of the list may never receive an update call at all.

---

_{If you want some "justification by example": I first encountered this pattern in the JUNG Graph Library, for example, in the SpringLayout class and others. When I first saw it, I cringed a little, because at the first glance it looks horribly hacky and dangerous. But here, the justification is the same: The process has to be as fast as possible, and modifications to the graph structure (which would cause the exception) are rare. Note that the JUNG guys actually do recursive calls to the respective method when the ConcurrentModificationException happens - simply because they can't always assume the method to be called constantly by another thread. This, in turn, can have nasty side-effects: If another thread does constant modifications, and the ConcurrentModificationException is thrown each time when the method is called, then this will end with a StackOverflowError... But this is not the case for you, fortunately.}