Java Synchronized Collections - what is the value?

Question

The following code triggers a ConcurrentModificationException very, very quickly:

import java.util.*;
public class SynchFail {
    static List<Integer> LIST = new ArrayList<Integer>();
    public static void main(String[] args) {
        new Thread(new Runnable() {
            @Override
            public void run() {
                while (true) {
                    LIST.add(1);
                }
            }}).start();
        new Thread(new Runnable() {
            @Override
            public void run() {
                while (true) {
                    List<Integer> syncList = Collections.synchronizedList(LIST);
                    synchronized(syncList) {
                        for (Integer thisInt : syncList) {
                        }
                    }
                }
            }}).start();
    }
}

... whereas the following behaves as it should:

import java.util.*;
public class SynchSucceed {
    static List<Integer> LIST = new ArrayList<Integer>();
    public static void main(String[] args) {
        new Thread(new Runnable() {
            @Override
            public void run() {
                while (true) {
                    synchronized(LIST) {
                        LIST.add(1);
                    }
                }
            }}).start();
        new Thread(new Runnable() {
            @Override
            public void run() {
                while (true) {
                    synchronized(LIST) {
                        for (Integer thisInt : LIST) {
                        }
                    }
                }
            }}).start();
    }
}

... my understanding was that synchronized collections were to prevent ConcurrentModificationExceptions in situations like this (but clearly they do not).

Given this: where should I make use of these ?

can you pls explain this? `but clearly they do not` - as you are getting exception because you are using ArrayList and there is nothing related to synchronized collections — tym32167, Nov 18 '21 at 18:39

Sweeper · Accepted Answer · 2021-11-18T18:59:16.927

In the first code snippet, you have not followed the instructions in the documentation of synchronizedList:

In order to guarantee serial access, it is critical that all access to the backing list is accomplished through the returned list.

In the other thread, you are adding to the list via the original LIST, not the "returned list". LIST is just a normal ArrayList and calling add on it won't acquire any locks or anything like that, so add could still be successfully called while the iteration is in progress.

If you did:

final static List<Integer> LIST = Collections.synchronizedList(new ArrayList<>());
public static void main(String[] args) {
    new Thread(new Runnable() {
        @Override
        public void run() {
            while (true) {
                LIST.add(1);
            }
        }}).start();
    new Thread(new Runnable() {
        @Override
        public void run() {
            while (true) {
                synchronized(LIST) {
                    for (Integer thisInt : LIST) {
                    }
                }
            }
        }}).start();
}

Then it wouldn't throw a CME. When you call add on a synchronised list, it tries to acquire the intrinsic lock on LIST. If iteration is in progress, the lock would have been already held by the other thread (since you did synchronized (LIST) { ... } there), so it will wait until the iteration is over. Compare this with the second code snippet, and notice how this saves you from writing an extra synchronized (LIST) {} block around the add call.

But I think, it’s ok to question the usefulness of an API that allows to omit the `synchronized (LIST) {}` block around a single call while still requiring the developer to take care for every compound action (and in real life, almost every operation will be a compound action). It’s too tempting to omit the `synchronized (LIST) {}` block for a single-call operation and the next time, a developer comes back, to add a pre-test or some other aspect making a second call, ending up at the “check-then-act” family of bugs. Always using a `synchronized (LIST) {}` block reminds the reader to take care… — Holger, Nov 29 '21 at 12:58
@Holger I do agree that the usefulness of `Collections.synchronizedList` is rather limited, though not _completely useless_. The question was asking about "what is the value (of synchronised lists)", so I just answered in that direction. — Sweeper, Nov 29 '21 at 13:11
Yes, of course. Discussing “how to use it correctly” is more important than discussing the general usefulness. I just felt that it’s worth noting that the OP’s second code snippet is not wrong and might be even preferable over the `Collections.synchronizedList`, as it doesn’t create a false impression of thread safety. — Holger, Nov 29 '21 at 13:21

score -1 · Answer 2 · answered Nov 18 '21 at 18:45

-1

Couple of things:

If you need sychronized access to an ArrayList you should use Vector instead. It does the same thing but its methods are syncrhonized.
in your case, the 2nd snippet works because you are syncing over the same object LIST in both threads

answered Nov 18 '21 at 18:45

Neddo

9
4

No, don't use `Vector`. Either use `synchronizedList` correctly or use something like `CopyOnWriteArrayList`. – chrylis -cautiouslyoptimistic- Nov 18 '21 at 19:07
Thx for the feedback, but the above was an example of a larger bit of code I'm working on, an in that case Vector is absolutely not an option. – Dave Carpeneto Nov 18 '21 at 19:28

Java Synchronized Collections - what is the value?

2 Answers2