How to synchronize Map between one r/w Thread and one read-only Thread?

Question

I have a synchronized Map (via Collections.synchronizedMap()) that is read and updated by Thread A. Thread B accesses the Map only via Map.keySet() (read-only).

How should I synchronize this? The docs say that keySet() (for a Collections.synchronizedMap) "Needn't be in synchronized block". I can put Thread A's read/write access within a synchronized block, but is that even necessary?

I guess it seems odd to me to even use a synchronized Map, or a synchronized block, if Map.keySet doesn't need to be synchronized (according to the docs link above)...

Update: I missed that iteration of the keySet must be synchronized, even though retrieving the keySet does not require sync. Not particularly exciting to have the keySet without being able to look through it, so end result = synchronization required. Using a ConcurrentHashMap instead.

Answer 1

To make a truly read/write versus read/only locking Map wrapper, you can take a look at the wrapper the Collections uses for synchronizedMap() and replace all of the synchronized statements with a ReentrantReadWriteLock. This is a good bit of work. Instead, you should consider switching to using a ConcurrentHashMap which does all of the right things there.

In terms of keySet(), it doesn't need to be in a synchronized block because it is already being synchronized by the Collections.synchronizedMap(). The Javadocs is just pointing out that if you are iterating through the map, you need to synchronize on it because you are doing multiple operations, but you don't need to synchronize when you are getting the keySet() which is wrapped in a SynchronizedSet class which does its own synchronization.

Lastly, your question seemed to be implying that you don't need to synchronize on something if you are just reading from it. You have to remember that synchronization not only protects against race conditions but also ensures that the data is properly shared by each of the processors. Even if you are accessing a Map as read-only, you still need to synchronize on it if any other thread is updating it.

Answer 2

The docs are telling you how to properly synchronize multi-step operations that need to be atomic, in this case iterating over the map:

Map m = Collections.synchronizedMap(new HashMap());
      ...
Set s = m.keySet();  // Needn't be in synchronized block
      ...
synchronized(m) {  // Synchronizing on m, not s!
    Iterator i = s.iterator(); // Must be in synchronized block
    while (i.hasNext())
        foo(i.next());
}

Note how the actual iteration must be in a synchronized block. The docs are just saying that it doesn't matter if obtaining the keySet() is in the synchronized block, because it's a live view of the Map. If the keys in the map change between the reference to the key set being obtained and the beginning of the synchronized block, the key set will reflect those changes.

And by the way, the docs you cite are only for a Map returned by Collections.synchronizedMap. The statement does not necessarily apply to all Maps.