Immutability doesn’t guarantee thread safety

With the introduction of the new Java Memory Model (JMM) in Java 5 (JSR-133) a lot of unclearities have been removed from Java regarding immutable classes and thread safety. Under the new JMM it is now perfectly clear that the immutable nature of a class doesn’t guarantee that it is thread safe. Take a look at the following class:

public class MyInt{
    private int x;

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

    public int getValue(){
        return x;
    }
}

Reorderings and Visibility

MyInt is not thread safe by nature, even though it is immutable. I hope you wonder why. To understand what can go wrong, I have to introduce 2 concepts:

1. reorderings: the compiler (Javac, JIT, CPU etc) is allowed to reorder instruction to improve performance. To name a few techniques that rely on reordering:
   1. locality: reads/writes to variables can be grouped to increase the chance of cache hit
   2. dynamic scheduling: to prevent that a CPU has to wait (and do nothing) for instructions in the pipeline (maybe memory access is delaying), it can execute unrelated instructions.

   There are more techniques, and you are never sure which techniques are used because Java is platform independent. You just need to remember that instructions can be reordered unless this is prevented.

2. visibility: inside a single thread all reads/writes to a variable need to be visible, but no guarantees are made with normal variables that changes made in by some thread to a variable, are visible in another thread. Only when there is a happens-before edge between a write and a read, you are sure that the change is visible. The reason why this strange behavior exists is performance. Because of the strong memory coherence most CPU have, visibility problems normally won’t occur that often.

So what can go wrong with MyInt?

Constructors are not treated special by the compiler (JIT, CPU etc) so it is allowed to reorder instructions from the constructor and instructions that come after the constructor. In the case of the MyInt it is allowed that the assignment of a newly created instance of of MyInt is assigned to a variable before the constructor has run. This reordering is never visible in the executing thread (within-thread if-serial semantics), but if another thread is going to read x, it could see a partially created MyInt and sees 0 (the default value of an int member variable) at one moment, and the next moment it sees the correct value for x. This problem can be fixed by making x final because this prevents reordering. Under the old JMM, making the field final even is not a solution.

Another problem is that the write to x by the constructing-thread, and the read from x by another thread, doesn’t need to have a happens-before edge. If this edge is missing, no guarantees are made that a change made in one thread, is visible in another thread (in other words, x is not safely published). The consequence is that the other thread doesn’t need to see the constructed value (ever!) and still sees the initial value. This visibility problem can be solved by making x final (volatile would also be a working solution).

Conclusion

Not all immutable classes are thread safe by definition, only proper created ones are. The simplest thing that can be done is making variables final, if these can be set in the constructor. In case of setters, reordering and visibility problems can be prevented by making the field volatile or make sure that the field is always used in a synchronized context (eg synchronized getter/setter or a Lock).

The new JMM has a new and very useful property: save hand off. It allows objects with visibility and reordering issues to be used safely in a multi threaded environment because reorderings and visibility problems are prevented. Not properly created immutable classes can be passed through such a structure without worrying about these issues. But if you have the chance, try to do it good from day one.

If you want to learn more about the JMM, I suggest reading “Java Concurrency in Practice”.

Interruptible: To be or not to be

At the moment I’m working on the last issues for a first release of my concurrency library (called Prometheus). One of the things that annoyed me, were the large amount of methods that only differ in being or not being interruptible. Most blocking methods in this library had the following form:

E foo() throws InterruptibleEx;

E fooUninterruptibly();

And in most cases timed versions are also needed (untimed versions or more likely to cause deadlocks and other liveness problems):

E tryFoo(long timeout, TimeUnit unit)throws InterruptibleEx, TimeoutEx;

E tryFooUninterruptibly(long timeout, TimeUnit unit)throws TimeoutEx;

So for every blocking action there were 4 different variations, and this caused a lot of repetition in tests, implementation and documentation. Another consequence is that a library gets more complicated to use because there are a lot of alternatives to choose from, even though in 99% of the cases it is better to use one of the interruptible versions. Making a call uninterruptible maybe feels good because you don’t have to deal with the checked InterruptedException anymore. But it also prevents a blocking thread from being interrupted and this this could lead to other problems like a ThreadPoolExecutor that doesn’t shut down when the ThreadPoolExecutor.shutdownNow() is called.

One of the problems I had in the beginning while implementing uninterruptible timed methods, is that the Condition has no uninterruptible timed wait method. That is why I added an implementation to my library. I finally posted a question on the concurrency mailing list why this method was missing. David Holmes (one of the guys behind JSR-166 and one of the authors of ‘Java Concurrency in Practice’) replied that there was a reason why that method was missing:

If you are able to deal with a timeout, you probably are able
to deal with an interrupt as well.

Uninterruptible blocking methods

This started me thinking. First of all, it would be nice if I could drop most uninterruptible methods, because you don’t want to use them in most cases anyway. Only in cases where an InterruptedException leaves the system in an inconsistent state, an uninterruptible version is a better alternative. I was walking to my work, and it finally occurred to me: dealing with an interruptible section doesn’t have to be added to all components because it could be something ‘reusable’, example:

E fooUninterruptible(){
	UninterruptibleSection<E> section = new UinterruptibleSection<E>(){
		E originalsection()throws InterruptedEx{
			//interruptible call
			return fooObject.foo();
		}
	};
	section.execute();
}

The fooUninterruptibly is an uninterruptible version of the interruptible foo method. The UninterruptibleSection is responsible for dealing with the InterruptedException that is thrown by the original section of code. So whenever an uninterruptible version is needed, just wrap it in an UninterruptibleSection.

For those that are interested, this is the implementation of the UninterruptibleSection:

public abstract class UninterruptibleSection<E> {

    protected abstract E originalsection() throws InterruptedEx;

    public E execute() {
        boolean restoreInterrupt = Thread.interrupted();
        try {
            while (true) {
                try {
                    return originalsection();
                } catch (InterruptedException ex) {
                    restoreInterrupt = true;
                }
            }
        } finally {
            if (restoreInterrupt)
                Thread.currentThread().interrupt();
        }
    }
}

(Thanks David for pointing out the assertion failure).

Uninterruptible timed blocking methods

For timed calls I created a similar version, so that you get the same behavior as with the UninterruptibleSection. The main difference is that the timeout is taken into account, but other than that it works just the same. The question remains which one to choose, retrying until a timeout occurs (the behavior my solution provides), or failing immediately when a call is interrupted (the behavior David suggested). Personally I don’t know which one to choose because both of them make sense. In my case there is some chance that an action completes, and the solution David suggests, is more responsive to interrupts.

I refactored the code so it uses an UninterruptibleSection, the code for the uinterruptible timed-await for condition after refactoring looks like this:

public static long awaitNanosUninterruptibly(
	final Condition condition,
	long timeoutNs
	)throws TimeoutException {
    if (condition == null) throw new NullPointerException();

    TimedUninterruptibleSection<Long> section =
    	new TimedUninterruptibleSection<Long>() {

        protected Long originalsection(long timeoutNs)
                    throws InterruptedException, TimeoutException {
            timeoutNs = condition.awaitNanos(timeoutNs);
            if (timeoutNs <= 0)
                throw new TimeoutException();
            return timeoutNs;
        }
    };

    return section.tryExecute(timeoutNs, TimeUnit.NANOSECONDS);
}

Conclusion

With the addition of the UninterruptibleSection and the TimedUninterruptibleSection I was able to remove a lot of code and simplify the synchronization behavior of interfaces and classes without losing functionality. And with Java 7 and the closure support, the syntactic ugliness can even be softened.

Spring and visibility problems

I posted a blogentry on Spring and visibility problems on the company website. If you want to read the post, you can find it here:

http://blog.xebia.com/2007/03/01/spring-and-visibility-problems