Friday, April 17, 2009

The power of the final keyword - updated

Picture courtesy of Okko Pyykkö@flickr
These two articles of Alex Miller:clearly explain why using final fields in concurrent programs is extremely powerful and safe (thread-safe).

Take a look a this simple example:

public class SomeClass implements Runnable {
private final LinkedHashMap map;

public SomeClass(LinkedHashMap copiedMap) {
map = copiedMap;
preProcessMap();
}

private void preProcessMap() {
//some non-trivial and not-so-short computations
}

// A read method
public Object get(Object key) {
return map.get(key);
}

public void run() {
//assume that in this method only
//unsynchronized read operations on map may occur
//any unsynchronized write operation is not thread-safe
}
// etc...
}

public class Main {
public static void main(String[] args) {
LinkedHashMap map = // map creation
// map initialization
new Thread(new SomeClass(map)).start();
// etc.
}
}
UPDATE:
I preserved the original version of the code but after some thoughts and thanks to some comments (thank you dear readers) I conclude that this example is not 100% correct. Before Thread.start() could be started the object has to be fully initialized and initialization will be completed only AFTER the SomeClass object is created. This way final keyword doesn't add any value there. On the other hand I meant this piece of code:

public class Main {
public static void main(String[] args) {
LinkedHashMap map = // map creation
// map initialization
final ExecutorService service = Executors.newFixedThreadPool(1);
service.submit(new SomeClass(map));
// etc.
}
}
In such case I'm pretty sure you cannot assume that e.g. second processor (in a multi-core platform) will wait before executing newly scheduled task until the object of class SomeClass is fully initialized, right? Correct me if I'm wrong because I feel have some mess in my brain now ;)
End of UPDATE

Whatever happens in the preProcessMap() method you can be sure that LinkedHashMap will be copied and preprocessed BEFORE starting the run() method by the new thread. This way any READ (get) operation on this map in all threads will be safe and will operate on completely and correctly initialized map object.

If only you remove final keyword from LinkedHashMap declaration you cannot be sure our previous assumption. It may happen that the thread will be started BEFORE copying the map and if in run() method you process this map in any way you may get NPE.

Let me use a bottle metaphor :) For me final keyword is like the guarantee that you closed the bottle of whisky before passing it to your crazy friend who likes juggling them. If you ensure that the bottle is closed (final) before passing it to your friend you can be sure he will not spill any of the precious whisky on the floor. On the other hand if you start passing the bottle to him while trying to close it you're not sure whether it's fully closed while he starts juggling it or not. It may be closed 100% of time when you meet in your place but when you go to your other friend's party he may spill very expensive whisky and break the bottle in the same time (test and production environments metaphor this time).
Conclusion? Close the bottle before giving it to anybody else.

I will not be original and simply copy Alex Miller's advice:
Final fields are ever so important and useful. Whenever possible, strive to make your fields final. One tip for this is to enable a Save Action in Eclipse (or your IDE of choice) that automatically attempts to make fields final if possible so you can’t forget!

10 comments:

Shantanu Kumar said...

This one is a really bad example. What if you accidentally write a map.put(...) statement in the code somewhere? Or what if a maintenance programmer does that?

final is fine for primitives. But for objects you need immutable data types (e.g. Collections.unmodifiableMap(...)).

Ram said...

Agree with Shantanu, it is too simplistic to assume final is be all and end all for being thread safe.

It will be safer to say synchronized is what makes things thread safe.

Of course that comes with a cost. We can reduce the cost by reducing the scope of synchronization through some strategies.

Using the appropriate concurrent utils in jdk5+ is one way to achieve this.

Using thread local variables is another.

Limiting all modifications of shared object to a dedicated thread is yet another.

Ensuring every thread has its own copy of data to work on is yet another - maybe this is truly what you wanted to get at here.

Remember that a completely immutable object is not very useful for most processing purposes.

abies said...

Thread.start is synchronized. Before that new thread is not existing, so it cannot have a copy of old memory from before that moment.

I think that your particular example will work perfectly well without final keyword.

Which clearly shows that concurrency is NOT easy ;)

Przemysław Bielicki said...

@Shantany & Ram - you're partially right :) I forgot to add very important detail that it's safe only fore reads i.e. when we assume map.put(...) statement will never occur.

@abies - synchronization of start() method only guarantees that "It is never legal to start a thread more than once." not that the initialization of the object in another thread will finishe before starting this thread. Look - in the code I provided I create new thread together with creating the object of SimpleClass. You cannot be sure what will be done first because the second thread is being created.

Przemysław Bielicki said...

ERRATA: when we assume map.put(...) statement will never occur AFTER the object is created. On the other hand preProcessMap() can do whathever reads and writes it likes because until the constructor is finished final fields are safe to modify.

abies said...

Sorry to be nitpicking, but multithreading programming is about details:

"Look - in the code I provided I create new thread together with creating the object of SimpleClass."

No, you are creating new thread after SimpleClass has finished initialization. Even that doesn't matter so much, as it is the 'start' method which is really spawning a new execution thread, which is called even after Thread constructor.

From
http://gee.cs.oswego.edu/dl/cpj/jmm.html

"Thread.start has the same memory effects as a lock release by the thread calling start, followed by a lock acquire by the started thread."

On lock release, thread view of data is flushed to common memory. On lock acquire, thread view of data is synced from common memory. This means, that anything put in code of calling thread before .start is called will be fully visible to new thread. As SimpleClass is fully finishes initialization even without final keyword, it will be visible to spawned thread.

Only tricky part with thread creation is if you spawn a new thread from constructor of the object and let 'this' pointer escape to the new thread. In such case, you are out of luck even if final keyword is used.

I think that
http://moonbase.rydia.net/mental/blog/programming/java/the-java-memory-model-for-programmers
got it right in the small table in the middle of the post. You can see there
Writes up to and including…
a call to Thread.start
...are made visible to…
the newly started thread.

You may want to browse the rest of the cases to get better understanding of the relations between threads.

Przemysław Bielicki said...

You sound resonable and this is the reason why I have to confront your opinion with this comment: http://java2jee.blogspot.com/2008/11/java-concurrency-is-easy-combining.html?showComment=1226948760000#c4144762493589566546

Any ideas? Does it make a difference when you create new thread and when you submit your task to the executore service?

Mike K said...

abies is entirely correct. The JLS specifies that on Thread.start() happens before any other action on that thread. Thus, with or without final in, the new Thread will see the correct data for SomeClass. Since SomeClass and the Thread are constructed on the Main thread before the call to start() happens, then I would be interested to know how you think a NullPointerException can happen on the newly spawned Thread.

Ram said...

"@Shantany & Ram - you're partially right :) I forgot to add very important detail that it's safe only fore reads i.e. when we assume map.put(...) statement will never occur."

If you never intended to make your map modifiable, the example makes absolutely no sense. I agree with abby that there was not a problem to begin with that you are trying to solve with your immutable class.

abies said...

There is a difference between creating new thread and letting thread pool to handle the runnable. In first case, you are invoking explicit memory synchronization barrier, in second it is not really guaranteed (might be implementation dependent I think).

In Sun JDK implementation, I think it will work even with executor pool. Worker class is doing the runLock.lock(); before execution of actual Runnable starts (so it is loading state from common memory). Caller thread on the other hand is synchronizing in submit, which is calling execute, which is calling addIfUnderCorePoolSize, which will flush there with unlock before using the thread to run the runnable OR offering it to blocking queue from which Worker thread will retrieve it, also creating synchronization point.

I'm not sure if this is possible to create Executor service which would allow for different behaviour and still be compatible with all the specs - but it is also not clearly specified that memory is synchronized before passing data to worker threads. In such case, it is always better to stay on the safe side. In fact, it is always better to stay on the safe side, even in example you gave here, because it reduces amount of magic dependencies in code and allows nasty surprises when code is later refactored by somebody else. Plus final fields make things easier to understand even in single threaded code ;)

Easiest example of need for final is non-synchronized static variable which is pointing to some singleton-like data. If you don't care about possible multiple initializations, you can do lazy initialization of it from calling thread if needed. In such case, some other thread running 'later' might see null and will initialize it again - but it is not an issue. What it the issue is that if you use non-final fields in it, it can see a reference to the 'singleton', but still half-initialized/null contents, even if you initialized it fully before assigning to static variable.