Does Collections.synchronized map makes Iterator threadsafe

Question

There are two threads in a system. One is a reader thread and another is a writer thread.

The map is synchronized using the following code.

Map<String,ArrayList<String>> m = Collections.synchronizedMap(new HashMap<String,ArrayList<String>())

The reader thread obtains an Iterator on the values of the map and at the same time writer thread modifies the map.

So, my question is will the Iterator throw ConcurrentModificationException?

How can anything modify the map? You have not kept a reference to it. — Bohemian, Oct 17 '15 at 19:31
Well, I am about to design a system. I was thinking of using ReadWriteLock. That's why I wanted to explore all possible scenarios. — Touchstone, Oct 17 '15 at 19:35
The code in your question is 100% threadsafe because it is not possible to modify the map - there is no direct reference to the map. — Bohemian, Oct 17 '15 at 19:59
@Bohemian, a synchronized view is not immutable. The underlying map can be modified simply by calling `m.put(String, List);` — jaco0646, Oct 17 '15 at 21:57

zapl · Answer 1 · 2015-10-17T19:31:56.377

Maybe. It's not safe to do so. The documentation says

It is imperative that the user manually synchronize on the returned map when iterating over any of its collection views

Collections.synchronized... makes single method calls atomic so they don't need further synchronization. But iteration is more than a single method call so it needs extra synchronization. Below is an example

    Map<String, String> shared = Collections.synchronizedMap(new HashMap<>());

    new Thread(() -> {
        while (true) {
            synchronized (shared) {
                for (String key : shared.keySet()) {
                    System.out.println(key);
                }
            }
            try {
                Thread.sleep(1000);
            } catch (Exception e) {
                break;
            }
        }
    }).start();

    new Thread(() -> {
        while (true) {
            try {
                // this is atomic
                shared.put(UUID.randomUUID().toString(), "Yo!");
                Thread.sleep(1000);
            } catch (Exception e) {
                break;
            }
        }
    }).start();
}

So, I guess I need to use synchronization for both reading and writing right? — Touchstone, Oct 17 '15 at 19:23
@Touchstone depends on what you do during writing. Single method calls no, if more than one needs to be atomic yes, see edit in answer — zapl, Oct 17 '15 at 19:32

Kumar Abhinav · Answer 2 · 2015-10-17T19:28:58.140

1

Yes,the Iterator may still throw ConcurrentModificationException as it is not related with synchronization(although its name suggests so).The Iterator tries to detect structural modifications(addition or deletion of Objects) in a best effort attempt and is irrespective of whether the operations of a List are synchronized or not. Once an Iterator has been obtained through List.iterator() or List.listIterator(),any changes done to the List (apart by the Iterator itself) will throw a CME exception in best effort basis. The only way you can ensure that ConcurrentModificationException is not thrown is either by having your Reader operations complete first and then writer operations(or vice-versa) or by using a fail-safe Iterator from ConcurrentHashMap

Map hashmap = new HashMap<String,ArrayList<String>();
----

Map<String,ArrayList<String>> m = new ConcurrentHashMap<String,ArrayList<String>(hashmap));

ConcurrentHashMap is a fail-safe iterator and now you can focus on synchronizing reader-writer operations than worrying about ConcurrentModificationException.

edited Oct 17 '15 at 19:28

answered Oct 17 '15 at 19:16

Kumar Abhinav

6,565
2
24
35

So, I guess I need to use synchronisation for both reading and writing right? – Touchstone Oct 17 '15 at 19:23
Only synchronization wont be sufficient,you need to ensure that operations of iterators dont interleave on one another.Alternatively I suggest you use a fail-safe iterator from ConcurrentHashMap – Kumar Abhinav Oct 17 '15 at 19:25
But fail-safe iterator uses the deep-copy, that will result in dirty read!! – Touchstone Oct 17 '15 at 19:27
@Touchstone Then I suggest you should synchronize your Iteration operations right from obtaining it Map.values().iterator() to using that iterator – Kumar Abhinav Oct 17 '15 at 19:31
Note that the question needs clarification - The code in the question makes modification impossible because no reference is held to it. – Bohemian Oct 17 '15 at 19:33
yeah, that's what I was thinking about. But the problem is if I have more than one readers then synchronization will make the systems sequential i.e. single threaded. Because if I use synchronization on read operation then only one thread will read at a time. Should I go for ReadWriteLock? – Touchstone Oct 17 '15 at 19:34
@Touchstone Why will yo utry to iterate everytime after a single reader or writer operation.Could you put some more code as to what you are trying to acheive? – Kumar Abhinav Oct 17 '15 at 19:37

Does Collections.synchronized map makes Iterator threadsafe

2 Answers2