Concurrent access to unmodifiableMap

Question

@Singleton
@LocalBean
@Startup
@ConcurrencyManagement(ConcurrencyManagementType.BEAN)
public class DeliverersHolderSingleton {

    private volatile Map<String, Deliverer> deliverers;

    @PostConstruct
    private void init() {
        Map<String, Deliverer> deliverersMod = new HashMap<>();
        for (String delivererName : delivererNames) {
            /*gettig deliverer by name*/
            deliverersMod.put(delivererName, deliverer);
        }
        deliverers = Collections.unmodifiableMap(deliverersMod);
    }

    public Deliverer getDeliverer(String delivererName) {
        return deliverers.get(delivererName);
    }

    @Schedule(minute="*", hour="*")
    public void maintenance() {
        init();
    }
}

Singleton is used for storing data. Data is updated once per minute. Is it possible, that read from the unmodifiableMap will be a problem with the synchronization? Is it possible that it will occurs reordering in init method and link to the collection will published, but collection not filled completely?

I guess the crucial issue is whether return from a `@PostConstruct` method is "safe publication" of the data set up there. — Raedwald, Jul 04 '13 at 14:04
If we assume that it isn't, so half-constructed beans would be exposed, that would be quite a large design flaw that would bite you in the a** in multiple places. — Kayaman, Jul 05 '13 at 05:13
`deliverers` being volatile should prevent reordering and your class should work as expected (assuming that *getting deliverer by name* is a thread safe operation). — assylias, Jul 08 '13 at 14:18

score 5 · Accepted Answer · answered Jul 08 '13 at 14:27

The Java Memory Model guarantees that there is a happens-before relationship between a write and a subsequent read to a volatile variable. In other words, if you write to a volatile variable and subsequently read that same variable, you have the guarantee that the write operation will be visible, even if multiple threads are involved:

A write to a volatile field (§8.3.1.4) happens-before every subsequent read of that field.

It goes further and guarantees that any operation that happened before the write operation will also be visible at the reading point (thanks to the program order rule and the fact that the happens-before relationship is transitive).

Your getDeliverers method reads from the volatile variable so it will see the latest write operated on the line deliverers = Collections.unmodifiableMap(deliverersMod); as well as the preceding operations where the map is populated.

So your code is thread safe and your getDeliverers method will return a result based on the latest version of your map.

score 0 · Answer 2 · answered Jul 08 '13 at 07:00

0

Thread safety issues here:

multiple reads from the HashMap - is thread safe, because multiple reads are allowed as long as there are no modifications to the collection and writes to the HashMap will not happen, because the map is an unmodifiableMap()
read/write on deliverers - is thread safe, because all java reference assignments are atomic

I can see no thread-unsafe operations here.

I would like to note that the name of init() metod is misleading, it suggests that it is called once during initialization; I'd suggest calling it rebuild() or recreate().

answered Jul 08 '13 at 07:00

Dariusz

21,561
9
74
114

Is it possible that the reference will be published before the collection will be initialized because of reordering? – shurik2533 Jul 08 '13 at 10:12
@shurik2533 If it did, I believe there would be no point in programming at all - [*compilers are allowed to reorder the instructions in either thread, when this does not affect the execution of that thread in isolation*](http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html) – Dariusz Jul 08 '13 at 10:22
@Dariusz There are reorderings that would not affect execution in single threaded mode but would affect a multithreaded program. For example moving `deliverers = Collections.unmodifiableMap(deliverersMod);` before the loop. – assylias Jul 08 '13 at 14:21
@Dariusz Not really - the unmodifiable map is a view on the original map. If you change the original map itself, the unmodifiable one will reflect the change too. – assylias Jul 08 '13 at 15:58
@assylias you are right about the map. I still think that the `unmodifiableMap()` method call can not get reordered to be called before the loop. What if that method modified the map? Java could not possibly know that during compile time. – Dariusz Jul 08 '13 at 17:02
I agree that this specific reordering is not going to happen in practice but without proper synchronization it is difficult to reason about how a program is going to behave. In this case the volatile variable provides the necessary synchronization. Another possible problem without volatile would be that the reading code might never see the result of the assignment for example. – assylias Jul 09 '13 at 11:13
@assylias you are right, but the question was not about the visibility:) using `volatile` does not provide *synchronization*, it only ensures that the value read is always the current value. The OP correctly used it to solve that problem. – Dariusz Jul 09 '13 at 11:20

score 0 · Answer 3 · answered Jul 08 '13 at 16:58

According to the Reordering Grid found here http://g.oswego.edu/dl/jmm/cookbook.html, the 1st operation being Normal Store cannot be reordered with the second operation being Volatile Store, so in your case, as long as the immutable map is not null, there wouldn't be any reordering problems.

Also, all writes that occur prior to a volatile store will be visible, so you will not see any publishing issues.

Concurrent access to unmodifiableMap

3 Answers3