Use Development Aspects to Enforce Concurrency Idioms in Java Applications

Java 5 has given us a killer concurrency library in java.util.concurrent. Mustang will add more ammunitions to the already performant landscape - now it is upto the developers to use it effectively for the best yield. If you are not Doug Lea and have been getting your hands dirty with the Executors and Latches and Barriers of java.util.concurrent, I am sure you must have realized that killer libraries also need quality programmers to deliver the good. I have been thinking of ways get some of the concurrency goodies into existing Java applications, who have recently migrated to the Java 5 platform and have still been struggling with performance problems on multithreaded programs. Of course for production environments, we cannot redesign things from scratch, however ingenuous solution we promise to deliver. However, in one of these recently migrated applications, it was one of my charters to do a code review and suggest a path of least resistance that can potentially introduce some of the improvements of java.util.concurrent without any major redesign.

I decided to approach the problem by trying to find out some of the obvious concurrency related problems that Brian Goetze has been harping upon in his Java Theory and Practice columns in IBM developerworks. The codebase was huge and has evolved over the last 3 years under the auspices of a myriad of programmers - hence I decided to equip myself with the most important crosscutting weapon, a range of development aspects that can point me to some of the possible problem areas. There may be some false positives, but overall the strategy worked .. the following post highlights some of them with some representative code snippets for illustration purposes.

Safe Publication of Objects #1

Do not allow the this reference to escape during construction. One typical scenario where programmers make mistake is to start a thread from within a constructor. And as Brian Goetze has pointed out, it is a definite anti-pattern. I wrote a small aspect for detecting this :

public aspect TrackEscapeWithThreadStartInConstructor {
  pointcut inConstructor() : initialization(org.dg.biz.*.new(..));
  pointcut threadStart() : call(void Thread.start());

  before() : cflow(inConstructor()) && threadStart() {
    throw new RuntimeException(
      "possible escape of this through thread start in constructor");
  }
}

and Eclipse was quick to point me to the offending classes :



Safe Publication of Objects #2

Another anti-pattern when the this reference escapes from the constructor is when the programmer calls a non-static instance method from within the constructor of a class. Here is a small aspect that catches this anti-pattern :

public aspect TrackEscapeWithMethodCallInConstructor {
  pointcut inConstructor(Object o)
   : target(o)
    && withincode(org.dg.biz.*.new(..));

  pointcut callInstanceMethod()
   : call(!private !static * org.dg.biz.*.*(..));

  before(Object o) : inConstructor(o)
    && callInstanceMethod()
    && if (o.equals(thisJoinPoint.getTarget())) {

      throw new RuntimeException(
      "possible escape of this through instance method call in constructor");
  }
}

and Eclipse responds :



Using the New Concurrent Collections

Java 5 and Java 6 offer concurrent collection classes as definite improvements over the synchronized collections and can be used as drop in replacements in most of the cases. The older synchronized collection classes serialize all access to the collection, resulting in poor concurrency. The new ones (ConcurrentHashMap, CopyOnWriteArrayList etc.) offer better concurrency through locking at a finer level of granularity. If the traversal in the concurrent collections is the dominant operation, then these new collection classes can offer dramatic scalability improvements with little risk - for more information refer to Brian Goetze's excellent book on Java Concurrency in Practice.

I wrote the following aspect which pointed me to all possible uses of the synchronized collections in the codebase :

public aspect TrackSynchronizedCollection {
  pointcut usingSyncCollection() :
   call(public * java.util.Collections.synchronized*(..));

  declare warning
    : usingSyncCollection()
    : "consider replacing with Concurrent collections of Java 5";
}

We did a careful review and replaced many of those occurences with the newer collections - and we were able to achieve significant performance gains in some situations.

Handling the InterruptedException

In many places within the application that deal with blocking apis and multithreading, I found empty catch blocks as handlers of InterruptedException. This is not recommended as it deprives code higher up on the call stack of the opportunity to act on the interruption - once again refer to Brian Goetze for details.

However, a small aspect allows to get to these offending points :

public aspect TrackEmptyInterruptedExceptionHandler {
  pointcut inInterruptedExceptionHandler()
   : handler(InterruptedException+);

  declare warning
    : inInterruptedExceptionHandler()
    : "InterruptedException handler policy may not be defined";

  before() : inInterruptedExceptionHandler() {
    Thread.currentThread().interrupt();
  }
}

Development aspects are a great tool for refactoring and code review. I realized this first hand in the exercise that I did above and succeeded in identifying some of the anti-patterns of writing multi-threaded programs in Java and enforcing some of the common idioms and best practices across the codebase. In the above aspects, I have only scratched the surface - in fact developing a library of development aspects will be a great tool to the developer at large.