[concurrency-interest] question/bug in JCiP: Listing 4.12 ?

Mike mboard182 at gmail.com
Wed Apr 7 13:11:19 EDT 2010


In working through Java Concurrency in Practice, I have struggled to
understand a detail of Listing 4.12.  After researching it, I am wondering
if I've found a problem.

Specifically, it seams that the 'locations' map should not require
internal synchronization, since it is accessed only by readers.

This is an 'overkill' bug, not a true flaw, but I feel it is important as I
am trying to understand exactly what the minimal synchronization approach is
to provide the class' @ThreadSafe guarantee.

Note: everyone has slightly different terminology, I've attempted to quote
"" phrases I'm defining per JCiP.  Also, I've included relevant code at the

-PublishingVehicleTracker ('PVT' in my text below) is properly constructed
(no escaped this, etc)
-after construction its state cannot be modified, so PVT is logically
immutable.  ("effectively immutable")
--PVT's state is defined as the keys and *handles* to SafePoints only
--while the map itself is obviously mutable, it is only published through an
immutable wrapper
-access to the state of the SafePoint instances in the Map is handled via
-PVT's design (as writen, at least) imposes no invariants or restrictions on
valid state transitions for SafePoint instances

Therefore, after client code "safely publishes" an instance of PVT, it seams
to me that access to its underlying Map is threadsafe even if 'locations'
were just a HashMap.

It seems obvious that since access to 'locations' is read-only, there are no
race conditions  --  which leaves memory visibility as the issue.   think
the root of my confusion here is precisely how is visibility guaranteed.  Is
it from the client of PVT safely publishing the PVT instance?

It occurs to me that I am also relying on the assumption that the underlying
hashtable is deterministic and 'un'-lazy in its internals - technically I
guess there could exist an implementation that tunes itself,
possibly resizing, as it observes usage over time.  Without a syncronized
map in that case, races would be a possibility.

I tend first assume a problem with my understanding before a bug, so please
enlighten me here :)  Also, I have a pretty strong C++ background, with some
understanding of compilers, barriers, etc, if that helps your answers.

Thanks!  -mboard182

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

public class PublishingVehicleTracker {

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

public PublishingVehicleTracker(
                        Map<String, SafePoint> locations) {

        = new ConcurrentHashMap<String, SafePoint>(locations);

        = 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

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://cs.oswego.edu/pipermail/concurrency-interest/attachments/20100407/43990e98/attachment.html>

More information about the Concurrency-interest mailing list