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

Question

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);

}

Anemone

The fact that a single thread is a producer is not very important in this case. The problem happens when the consumer thread tries to read while the producer writes. – Daviddavida

@Daviddavida thx for you comment. This does not explain the synchronized block though? Whether the block was there or not, the situation that the newly created map is published while others read would arise anyway...or wouldn't it? Only one thread accessing the map once at the time would require the consumers to have a sync block as well, or any other lock mechanism... – Anemone

@Marko The volatile should address that issue. – Daviddavida

Answer 1

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.

Christman

That's what I thought. Basically, according to the article, in every multi-threaded situation you'd have to synchronize every single threaded method that alters a class field – Anemone

Perhaps, the article is based on pre-Java 5 knowledge, where the guarantees of a volatile variable were too weak. But even then, the guarantees of a synchronized block without the other thread using synchronized on the same object were too weak either. – Tessitura

@Tessitura They say it's about Java 5 at the end of the article "It's important to note that because of the intricacies of the Java memory model, the technique described here works only in Java 1.5 and later" – Christman

@Christman yes, I noticed it after writing the comment. I further noticed that this article on IBM seems to be a horribly distorted version of an article from Brian Goetz. Apparently the authors of the IBM article failed to recognize at which points their code differed from the other article. – Tessitura