Using for each syntax with Collections.synchronizedSet

Question

The problem is this. I have made a set

Set<User> users = Collections.synchronizedSet(new HashSet<User>())
...
for(User u : users){
   //do something with u
} 

Now, according to Java documentation.

It is imperative that the user manually synchronize on the returned sorted set when iterating over it or any of its subSet, headSet, or tailSet views.

 SortedSet s = Collections.synchronizedSortedSet(new HashSortedSet());
 ...
 synchronized(s) {
    Iterator i = s.iterator(); // Must be in the synchronized block
    while (i.hasNext())
       foo(i.next());
 }

I'm pretty sure that for each syntax uses iterators but I'm unsure whether or not I should wrap every for each loop with a synchronized block.

Another thing, my IDE (IntelliJ IDEA) keeps reporting that using a synchronized block over a field that is not final is unlikely to have useful semantics since different threads might be different objects, even when working on a same object.

Answer 1

As the for each uses the Iterator under the covers, you are still subject to a ConcurrentModificationException during the processing of a for each if your Set is structurally modified.

With regard to the warning that your IDE provides, if you are reassigning your s, there is a chance that different threads do see different objects when trying to synchronize on s. The use of volatile should help here.

Answer 2

1. whether or not I should wrap every for loop... No, you don't need to wrap every for loop with a synchronized block, infact you shouldnt even do that because it comes with a performance penalty. You need to wrap only those for loops that iterate over collections that maybe accessed by multiple threads concurrently.

2. my IDE (IntelliJ IDEA) keeps reporting.... In your particular case, it's not a problem, because if different threads have different instances of s, then obviously the iterators will be different too.