Recently I posted a question on Stack Overflow asking for the most common Java concurrency bugs. I’m going to recap and consolidate some of the most popular answers in a series of blog posts. If you’d like to continue adding answers or voting over there, the question is still open.
The #1 voted answer was a problem with storing mutable non-threadsafe classes in static variables and calling them (without synchronization) from multiple threads. Two of the most common examples seen in the wild are java.util.Calendar and java.text.SimpleDateFormat. Because these are heavy objects that often need to be set up with a format or locale, it’s oh so tempting to create it with a static initializer and stick the instance in a static field.
However, both of these classes use internal mutable state when doing date calculations or formatting/parsing dates. If they are called from multiple threads at the same time, that internal mutable state will do things unexpected and most likely give you wrong answers. Of course, this will only happen occasionally in most programs, so you might have this bug for a long time before you discover it.
This is a good example of a problem that JSR 310 is trying address with the new Date and Time API. At this point it’s uncertain whether JSR 310 will be complete soon enough to be part of Java 7, but I certainly hope it is as it relies heavily on immutable classes, which make this concurrency problem a non-issue.
There are a variety of ways to solve this problem:
- Immutable objects – as in JSR 310, the best way is to make your statics final and use immutable objects, guaranteeing there is no thread-safety issue.
- Thread-safe wrappers – another option is to wrap an unsafe object in a thread-safe wrapper. One example would be wrapping a Collections.synchronizedList() around a mutable ArrayList.
- Encapsulation – another option is to use an object that is already thread-safe, such as one of the java.util.concurrent collections.
- Thread confinement – if you can guarantee that only one thread will ever access the static field, then you might get away with it. This is a dangerous game to play though and you should properly document that the field is unsafe to use from multiple threads.
- Explicit synchronization – a relatively poor option relative to the ones above is to define a locking strategy to use when accessing the static field, for instance using a static Object as a lock. This is a poor option because it’s so easy for the code to drift and for someone to forget to follow the locking strategy. In this case, using encapsulation or a thread-safe wrapper is highly recommended.
In a greater sense, any kind of mutable static state is suspect – this is basically just using global variables which is a well-known source of problems. Singletons are another related area although the concurrency concerns there are usually focused on initialization. If your singleton has mutable state, you have the same need to protect all methods on the shared instance from being accessed safely from multiple threads.
To find problems these in your own code base, FindBugs is an excellent choice. It has special detectors for Calendar and SimpleDateFormat so it will find these common examples if you have them. It can also find inconsistent synchronization where you are sometimes but not always accessing a field under a lock. FindBugs also has a set of detectors specific to mutable static fields although they may or may not be smart enough to detect problems in your code.