Concurrent iteration and thread safety

Question

I'm reading B. Goetz Java Concurrency In Practice and now I'm at the section about thread-safe collections. He described the so-called "hidden iterators" which may throw ConcurrentModificationException. Here is the example he dispensed:

public class HiddenIterator{

    @GuardedBy("this")
    private final Set<Integer> set = new HashSet<Integer>();

    public synchronized void add(Integer i){ set.add(i); }
    public synchronized void remove(Integer i){ set.remove(i); }

    public void addTenThings(){
        Random r = new Random();
        for(int i = 0; i < 10; i++)
            add(r.nextInt());
        System.out.println("DEBUG: added ten elements to set " + set)
    }
}

Now, it's obviously that addTenThings() may throw ConcurrentModificationException as that printing set's content involves iterating it. But he provide the following suggestion for dealing with it:

If HiddenIterator wrapped the HashSet with a synchronizedSet, encapsulating the synchronization, this sort of error would not occur.

I don't quite understand it. Even if we wrapped set into a synchronized-wrapper, the class would still remain NotThreadSafe. What did he mean?

Answer 1

This is because Collections.synchronizedSet synchronizes every method, including toString. Indeed, if you tried to iterate over a wrapped set manually, you could get ConcurrentModificationException, so you have to synchronize manual iteration yourself. But methods that do hidden iterations already do it, so you don't have to worry about that at least. Here is the corresponding piece of code from the JDK sources:

public String toString() {
    synchronized (mutex) {return c.toString();}
}

Here, mutex is initialized to this in the constructor of the wrapper class, so it's basically synchronized (this).