Don't quite understand check and set in concurrency

Question

I don't entirely understand check and set behaviour.. here is an example:

    public static void main(String[] args) {

    List<String> list = Collections.synchronizedList(new ArrayList<>());

    String abc = "123";

    if (!list.contains(abc)) {
        list.add(abc);

        // ... some several lines of code

        list.remove(abc);

    }

}

What I don't quite understand...

So I understand that contains and add should be in a synchronized(list) block...

but what about remove?

if not, then what if it was:

if (list.contains(abc)) {
   list.add(abc);
   list.remove(abc);
}  

?

I understand that code doesn't make much sense but I just am using it as an example... Does remove have to be in the synchronized block here?

Answer 1

You should probably read Java Concurrency in Practice by Brian Goetz and others

Answer 2

Any time a collection can be accessed by multiple threads without locking at a higher level (e.g. on a per-transaction basis, rather than per-operation), there is a potential for things to go wrong. You can't perform operations conditionally on any property of the collection, because that property could change between testing and execution. You can't iterate over the collection, because it could change during the iteration process. All you can do is add items to it. Therefore, any time you find yourself wanting to use a synchronised collection, take a step back: what you're doing is probably the wrong way of solving your problem.

Synchronisation needs to be at a higher level than the collection, or you need to use a collection that implements the higher level logic your application needs, e.g. a queue.

For your specific example, the potential issue is that an object equal to the element being added could be added by another thread, causing the add operation in the code you quote to fail. The remove operation would then remove the element added by the other thread, which is likely not what you want (the code seems to have the intent of leaving the collection unchanged after it finishes, but changing temporarily during some operation).

Whenever possible, it is best to avoid sharing low-level access to mutable values between threads, and problems like this are the main reason why.