How to prevent nested synchronized blocks when iterating over a collection

In a multithreaded Java application I need to iterate over a collection of objects. Since both the collection and the objects could be modified by another thread while I iterate over them, I need to use synchronization.

However nested synchronized blocks are not recommended since they could lead to deadlocks. How would I solve this problem?

Collection<Data> dataCollection = something.getDataCollecti开发者_高级运维on();

synchronized ( dataCollection ) {
  for ( final Data data : dataCollection ) {
    synchronized ( data ) {
      data.doSomething();  // doSomething() changes object state
    }
  }
}

I think you can use CopyOnWriteArrayList instead of the outer synchronization.

A thread-safe variant of ArrayList in which all mutative operations (add, set, and so on) are implemented by making a fresh copy of the underlying array. This is ordinarily too costly, but may be more efficient than alternatives when traversal operations vastly outnumber mutations, and is useful when you cannot or don't want to synchronize traversals, yet need to preclude interference among concurrent threads

You can take a copy of the collection and only lock one object at a time.

Collection<Data> dataCollection = something.getDataCollection();
Collection<Data> copy;
synchronized ( dataCollection ) {
  copy = new ArrayList<Data>(dataCollection);
}

for (Data data : copy) {
    synchronized ( data ) {
      data.doSomething();  // doSomething() changes object state
    }
}

Can't believe nobody pointed out that the number one way to avoid synchronizing on the Data object is to have this object itself thread-safe! It's also the correct way of handling synchronization - if you know that your object will be accessed by multiple threads, handle synchronization the way you see fit inside the class, not in the code that may access it. You will also certainly be more efficient because you can limit synchronization to just the critical blocks, use ReadWriteLock, j.u.c.atomic, etc

Nested synchronization can lead to deadlock, but it doesn't have to. One way to avoid deadlocks is to define an order that you synchronize objects and always follow it.

If you always synchronize the dataCollection object before you synchronize the data objects, you won't deadlock.

Take a look at ReentrantReadWriteLock. With this class you can implement a lock that makes it possible for any number of non-modifying (reading) threads to access the shared property simultaneously, but only one modifying (writing) thread to access it at a time (all other readers and writers are blocked until the writing thread releases the write-lock). Remember to test your implementation thorougly, as wrong usage of the locks can still lead to race condition and/or deadlocks.

Whether you use CopyOnWriteArrayList as Bozho said or copy the list before iterating as Peter says should depend on how much you expect the list to be edited compared to iterated over.

Use CopyOnWriteArrayList when you expect the list to be iterated over far more than it is modified.

Use copying the list if you think it will be modified far more than it is iterated over.

These should be the first options because concurrency solutions should be simple unless unavoidable, but if neither situation applies you will need to pick one of the more complicated strategies outlined in the comments here.

Good luck!