preventing a reordering by JIT in Java

Question

I have a method that looks like it was reordered by the JIT.

public boolean finishRequest(boolean success) {
    if (success) {
        error = false;
        wayPoints.put(FINISH_SUCCESS_WP, System.currentTimeMillis());
    } else {
        wayPoints.put(FINISH_ERROR_WP, System.currentTimeMillis());
        error = true;
    }
    someThread.addToQueue(this);//...submit for further processing on separate thread
    return true;
}

The wayPoints container is a field of "this" which gets accessed by someThread. I ended up getting a ConcurrentModificationException in some line of code within someThread when it iterates through this' wayPoints, via getWayPoints().

To fix this I created a new immutable class that would get passed to someThread instead of "this".

public boolean finishRequest(boolean success) {
    if (success) {
        error = false;
        wayPoints.put(FINISH_SUCCESS_WP, System.currentTimeMillis());
    } else {
        wayPoints.put(FINISH_ERROR_WP, System.currentTimeMillis());
        error = true;
    }
    PLog plog = new PLog(wayPoints.toArray(new WayPoint[wayPoints.size()]));
    someThread.addToQueue(plog);//...submit for further processing on separate thread
    return true;
}

In the first piece of code the JIT can reorder 'someThread.addToQueue()' and 'wayPoints.put()' because from what the compiler could tell wayPoints is not being read from further into the current thread. But in the fixed code, since I am using wayPoints to construct my new PLog instance, have I now effectively prevented the JIT from re-ordering?

Reference.... http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.4.5

From the reading of section 17.4.5 of the JLS I presume I have created a legally binding happens-before that would prevent re-ordering since I am now reading from wayPoints after writing to it.... "If x and y are actions of the same thread and x comes before y in program order, then hb(x, y)."

thank you!

Answer 1

You are absolutely correct you have created a happens-before relationship. In general

a = X ; ( X is some constant or expression not involving a & b ) b = Y ; ( Y is some constant or expression not involving a & b )

above two can be reordered by the JIT

But a = X ; b = a * Y ;

Will not be reordered. Which is pretty much what you have done in your correction.

Other way to fix your problem would be to declare wayPoints volatile. If you declare wayPoints as volatile, write to volatile ( wayPoints) would not be reordered with other memory operations ( someThread.addToQueue() ) in your original program order.

Answer 2

What do addToQueue and the corresponding "removeFromQueue" do? They will need some sort of synchronization, in which case the exact functioning of method "finishRequest" would not matter. Your rearranged method is no more or less wrong than before, unless of course, "this" is already in the queue before finishRequest runs. If addToQueue is not synchronized with its counterpart, someThread can still see an incompletely constructed PLog.