CODEWITHSUNDEEP
javaconcurrencythread-safety
assylias
assylias

Reputation: 328785

Safe publication of java.util.concurrent collections

Is volatile redundant in this code?

public class Test {
    private volatile Map<String, String> map = null;

    public void resetMap() { map = new ConcurrentHashMap<>(); }

    public Map<String, String> getMap() { return map; }
}

In other words, does map = new ConcurrentHashMap<>(); provide any visibility guarantees?

As far as I can see, the only guarantee provided by ConcurrentMap is:

Actions in a thread prior to placing an object into a ConcurrentMap as a key or value happen-before actions subsequent to the access or removal of that object from the ConcurrentMap in another thread.

How about other thread safe collections in java.util.concurrent (CopyOnWriteArrayList, etc.)?

Upvotes: 6

Views: 747

Answers (6)

Marko Topolnik
Marko Topolnik

Reputation: 200236

This is not just about the references. Generally, without the volatile modifier other threads may observe a new reference to an object, but observe the object in a partially constructed state. In general, it is not easy to know, even after consulting the documentation, which objects are safe for publication by a data race. An interesting note is that the JLS does guarantee this for thread-safe immutable objects, so if the docs mention those two properties it should be enough.

ConcurrentHashMap is obviously not an immutable object, so that doesn't apply, and the docs don't mention anything about publication by a data race. By careful inspection of the source code we may conclude that it is indeed safe, however I wouldn't recommend relying on such findings without this property being clearly documented.

Upvotes: 4

irreputable
irreputable

Reputation: 45443

My impression is that Doug Lea's concurrent objects can be safely published by data race, so that they are "thread-safe" even if misused. Though he probably wouldn't advertise that publicly.

Upvotes: 0

assylias
assylias

Reputation: 328785

OK - I was able to construct an example that breaks (on my machine: JDK 1.7.06 / Win 7 64 bits) if the field is not volatile - the program never prints Loop exited if map is not volatile - it does print Loop exited if map is volatile. QED.

public class VolatileVisibility extends Thread {

    Map<String, String> stop = null;

    public static void main(String[] args) throws InterruptedException {
        VolatileVisibility t = new VolatileVisibility();
        t.start();
        Thread.sleep(100);
        t.stop = new ConcurrentHashMap<>(); //write of reference
        System.out.println("In main: " + t.stop); // read of reference
        System.out.println("Waiting for run to finish");
        Thread.sleep(200);
        System.out.println("Still waiting");
        t.stop.put("a", "b"); //write to the map
        Thread.sleep(200);
        System.exit(0);
    }

    public void run() {
        System.out.println("In run: " + stop); // read of reference
        while (stop == null) {
        }
        System.out.println("Loop exited");
    }
}

Upvotes: 0

Amit Deshpande
Amit Deshpande

Reputation: 19185

Memory Consistency Properties

A write to a volatile field happens-before every subsequent read of that same field. Writes and reads of volatile fields have similar memory consistency effects as entering and exiting monitors, but do not entail mutual exclusion locking.

Actions in a thread prior to placing an object into any concurrent collection happen-before actions subsequent to the access or removal of that element from the collection in another thread.

Upvotes: 0

Tomasz Nurkiewicz
Tomasz Nurkiewicz

Reputation: 340933

volatile is necessary here. It applies to the reference, not to what it references to. In other words it doesn't matter that an object is thread safe, other threads won't see the new value of map field (e.g. might see previously referenced concurrent map or null).

Moreover, even if your object was immutable (e.g. String) you would still need volatile, not to mention other thread-safe collections like CopyOnWriteArrayList.

Upvotes: 6

Peter Lawrey
Peter Lawrey

Reputation: 533750

volatile is not redundant as you are changing the reference to the map. i.e. ConcurrentMap only provides guarentees about the contents of the collection, not references to it.

An alternative would be

public class Test {
    private final Map<String, String> map = new ConcurrentHashMap<>();

    public void resetMap() { map.clear(); }

    public Map<String, String> getMap() { return map; }
}

How about other thread safe collections in java.util.concurrent (CopyOnWriteArrayList, etc.)?

Only the behaviour of the collection is thread safe. Reference to the collection are not thread safe, elements in the collection are not made thread safe by adding them to the collection.

Upvotes: 8

Related Questions