Synchronizing on an object and changing the reference

Question

Let's say I have an object as follows:

Map<String, String> m = new HashMap<>();

Then I synchronize on this object as follows and change its reference:

synchronize(m){
    m = new HashMap<>();
}

With this code, what happens to the lock on m? Is it still safe to update the new object represented by m? Or is the lock essentially on the old object?

Pilcher

It's not safe - you may get a race between 2 more threads one of which is already blocked awaiting the old m and another that obtains a lock on a new m. For that very reason the lock object is usually separated from the objects that hold your data. – Calumniate

Answer 1

From JLS 17.1:

The synchronized statement (§14.19) computes a reference to an object; it then attempts to perform a lock action on that object's monitor and does not proceed further until the lock action has successfully completed. After the lock action has been performed, the body of the synchronized statement is executed. If execution of the body is ever completed, either normally or abruptly, an unlock action is automatically performed on that same monitor.

Now the questions.

What happens to the lock on m?

Nothing. This is a little bit confusing. Actually, the thread is holding the lock on the object referenced by m at the time it was trying to acquire the lock. The assignment to m in the synchronized block does not automatically "switch" the lock that is being held by the executing thread.

Is it still safe to update the new object represented by m?

It's not safe. The write to m is not synchronized on the same lock.

Or is the lock essentially on the old object?

Yes

Studio

Answer 2

The lock is on the object, not on the variable.

When a thread tries to enter a synchronized block it evaluates the expression in parens after the synchronized keyword in order to determine what object to acquire the lock on.

If you overwrite the reference to point to a new object, then the next thread that tries to enter the synchronized block acquires the lock on the new object, so it can be the case that two threads are executing code in the same synchronized block on the same object (the one that acquired the lock on the old object may not be done when the other thread starts executing the block).

For mutual exclusion to work you need the threads to share the same lock, you can't have threads swapping out the lock object. It's a good idea to have a dedicated object you use as a lock, making it final to make sure nothing changes it, like this:

private final Object lock = new Object();

This way, since the lock object isn't used for anything else, there isn't the temptation to go changing it.

Memory visibility does not seem relevant here. You don't need to take visibility into account when reasoning about how swapping the lock creates problems, and adding code to let the lock object change in a visible way doesn't help to fix the problem, because the solution is to avoid changing the lock object at all.

Asseveration

Answer 3

To safely change reference to the object you can:

1. Use AtomicReference

   AtomicReference<Map<String, String>>
   

2. Use synchronized on object which contains this map or better on some other lock object.

   class A {
       private final Object lock = new Object();
       private Map<String, String> m = new HashMap<>();
   
       public void changeMap() {
           synchronized(lock){
               m = new HashMap<>();
           }
       }
   }
   

3. At least add volatile

   private volatile Map<String, String> m = new HashMap<>();
   

Also see other answers on this topic

1. Is reference update thread safe?
2. In Java, is it safe to change a reference to a HashMap read concurrently

Intonation

"Use synchronized on object which contains this map" --- I'd consider this to be not the best advice. – Calumniate

"At least add volatile" --- why do you need it if you have synchronized? – Calumniate

@Calumniate Of course it is not the best advice, this is why I have written better on some other lock object and provided and example on it. And of course you don't need to add volatile if you have synchronized. There are at least 3 different approaches to make reference thread-safe, there is no need to mix them. Was it not clear from my answer? – Intonation

It was not (is not) clear whether any of those advices are applied to what OP has (especially the #3) or imply that it should be rewritten from scratch. Otherwise - a great answer :-) – Calumniate

Answer 4

Your approach is not safe. You need to use the same lock among all coordinating threads to protect some resource (the map m in this case), but as you intuitively understood this fails here because the object m is constantly changing.

To be specific, once you write a new reference to m inside the critical section, another thread can enter the critical section (since they get a lock on the new Map, not the old one held by the other thread), and access the new partially constructed map.

See also safe publication.

Intellectuality