Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why would I place a synchronized block within a single-threaded method?

I stumbled upon this article on IBM - developerworks, and the code they posted had me raise some questions:

  1. Why is the building of the local variable Map wrapped within a synchronized block? Note that they implicitly say there is only one producer thread.

  2. Actually, why would this snippet require a synchronized block at all? A volatile variable should be enough for the job, as the newly created map is published only after being filled up.

  3. What is the point of only one thread synchronizing on a lock object?

The article mentions:

The synchronized block and the volatile keyword in Listing 1 are required because no happens-before relationship exists between the writes to the currentMap and the reads from currentMap. As a result, reading threads could see garbage if the synchronized block and the volatile keyword were not used.

And the comment in the code says:

this must be synchronized because of the Java memory model

I feel I'm dealing with multi-threading concepts outside of my understanding; I'd like someone with more expertise to point me in the right direction.


Here is the snippet taken from the article:

static volatile Map currentMap = null;   // this must be volatile
static Object lockbox = new Object();  

public static void buildNewMap() {       // this is called by the producer     
    Map newMap = new HashMap();          // when the data needs to be updated

    synchronized (lockbox) {                 // this must be synchronized because
                                     // of the Java memory model
        // .. do stuff to put things in newMap
        newMap.put(....);
        newMap.put(....);
    }                 
    /* After the above synchronization block, everything that is in the HashMap is 
    visible outside this thread */

    /* Now make the updated set of values available to the consumer threads.  
    As long as this write operation can complete without being interrupted, 
    and is guaranteed to be written to shared memory, and the consumer can 
    live with the out of date information temporarily, this should work fine */

     currentMap = newMap;

}

public static Object getFromCurrentMap(Object key) {
    Map m = null;
    Object result = null;

    m = currentMap;               // no locking around this is required
    if (m != null) {              // should only be null during initialization
         Object result = m.get(key); // get on a HashMap is not synchronized

    // Do any additional processing needed using the result
    }
    return(result);

}
like image 240
Marko Pacak Avatar asked Mar 16 '18 21:03

Marko Pacak


1 Answers

According to the Java memory model there is a happens-before relationship between a volatile write and a subsequent volatile read, which means that m = currentMap; is guaranteed to see everything that happened before currentMap = newMap;, the synchronized block is not needed.

Not only that, but it does absolutely nothing, the:

this must be synchronized because of the Java memory model

and

After the above synchronization block, everything that is in the HashMap is visible outside this thread

comments are wrong. According to the Java memory model there is a happens-before relationship only when both threads are synchronized; using it as in the article does absolutely nothing according to the JMM (some JVM implementation can do something and maybe some IBM JVM implementation in 2007 performed some kind of synchronization in this case, but JMM doesn't require it).

In conclusion, the IBM article is simply wrong.

like image 119
Oleg Avatar answered Oct 14 '22 18:10

Oleg