question about book example - Java Concurrency in Practice, Listing 4.12

Question

Hi, I am working through an example in Java Concurrency in Practice and am not understanding why a concurrent-safe container is necessary in the following code.

I'm not seeing how the container "locations" 's state could be modified after construction; so since it is published through an 'unmodifiableMap' wrapper, it appears to me that an ordinary HashMap would suffice.

EG, it is accessed concurrently, but the state of the map is only accessed by readers, no writers. The value fields in the map are syncronized via delegation to the 'SafePoint' class, so while the points are mutable, the keys for the hash, and their associated values (references to SafePoint instances) in the map never change.

I think my confusion is based on what precisely the state of the collection is in the problem.

Thanks!! -Mike

Listing 4.12, Java Concurrency in Practice, (this listing available as .java here, and also in chapter form via google)

/////////////begin code

@ThreadSafe
public class PublishingVehicleTracker {

private final Map<String, SafePoint> locations;
private final Map<String, SafePoint> unmodifiableMap;

public PublishingVehicleTracker(
                        Map<String, SafePoint> locations) {
    this.locations
        = new ConcurrentHashMap<String, SafePoint>(locations);
    this.unmodifiableMap
        = Collections.unmodifiableMap(this.locations);
}

public Map<String, SafePoint> getLocations() {
    return unmodifiableMap;
}

public SafePoint getLocation(String id) {
    return locations.get(id);
}

public void setLocation(String id, int x, int y) {
    if (!locations.containsKey(id))
        throw new IllegalArgumentException(
            "invalid vehicle name: " + id);
    locations.get(id).set(x, y);
  }
}

// monitor protected helper-class

@ThreadSafe
public class SafePoint {

@GuardedBy("this") private int x, y;

private SafePoint(int[] a) { this(a[0], a[1]); }

public SafePoint(SafePoint p) { this(p.get()); }

public SafePoint(int x, int y) {
    this.x = x;
    this.y = y;
}

public synchronized int[] get() {
    return new int[] { x, y };
}

public synchronized void set(int x, int y) {
    this.x = x;
    this.y = y;
}

}

///////////end code

Answer 1

The method setLocation does indeed modify the contents of the map.

Update: I discussed the issue with a coworker and in the end we came to agree with @Zwei Steinen's conclusion (+1 to him :-).

Note that apart from what he mentioned, visibility is a concern too. However, in this case the map is declared final, which guarantees this.

Answer 2

You are right. I think it is an error in JCiP. If you want to be double sure, I suggest you post it to (its) mailing list at: http://gee.cs.oswego.edu/dl/concurrency-interest

Like you said, the map is not modified; modification of the value does not result in any "writes" on the map.

In fact, my production code does exactly what you suggest, and I have asked question on the said mailing list about that code. One of the author of JCiP told me it is okay to use a read-only hashmap for the container.

Here is the simplified version (no null check etc.) of my code (I ended up using google-collection's ImmutableMap.) :

class Sample {
    private final ImmutableMap<Long, AtomicReference<Stuff>> container;

    Sample(){
        this.container = getMap();
    }

    void setStuff(Long id, Stuff stuff){
        AtomicReference<Stuff> holder = container.get(id);
        holder.set(stuff);
    }
}

It has worked perfectly under extreme load, for an extended time.

Answer 3

@Zwei, thanks for the suggestion - Joe Bowbeer & Tim Peierls (JCiP coauthors) confirmed that, technically, an ordinary hash map would suffice in this case. (although any real-world design would probably have extra requirements that would necessitate the concurrent map)

The reasoning is that the underlying map is
-transitively reachable from a final field
-not changed since published via 'getLocations()' (actually its never changed after ctor, but that's not necessary)
-properly constructed

Joe pointed me to a couple very good blog posts explaining more about what java immutability really means:

http://jeremymanson.blogspot.com/2008/04/immutability-in-java.html
http://jeremymanson.blogspot.com/2008/07/immutability-in-java-part-2.html

I'd refer readers there for a complete explanation.

Thanks!