The Beauties and Pitfalls of ThreadLocals

ThreadLocal must be one of the Java features, which not many developers have used. It doesn't happen so often, that you stand in front of a problem, for which ThreadLocal is the best solution. In the current game I am developing, I have however decided to use a ThreadLocal to store per-thread slash per-request context information. Actually this is one of the use cases that Brian Goetz wrote about in his great article about ThreadLocals.

Other applications for ThreadLocal include storing or accumulating per-thread context information for later retrieval.


This game uses the Spring Framework and the Servlet which is handling all requests, is a Spring Bean. When the request comes in, the Servlet will delegate handling of the request data to something that we call a RpcHandler. Once the appropriate RpcHandler is selected by the Servlet, the request has to go through 3 stages and each stage is mapped to a method in the RpcHandler.


// stage 1
public void readRequest(final DatatypeInput in) {

}

// stage 2
public void performOperation() {

}

// stage 3
public void writeResponse(final DatatypeOutput out) {

}


First the request data is read, then the RpcHandler is performing it's main operation and finally some response data might be written. In other games developers have decided to create a new instance of the RpcHandler subclass for each request. This makes it easy as the per-request context information can be stored in private fields of the RpcHandler. The negative part is that this approach is using more memory as there will be 1 instance of RpcHandler per request. This might sound acceptable if the RpcHandler was simple and lightweight but often they contain further object-trees. Because our game was Spring based, I was striving for a solution were we only had 1 instance per RpcHandler sublass. Each instance is injected as a Spring Bean into the Servlet. Since the RpcHandler interface could not be changed, so that per-request context information was given as a parameter to the 3 methods above, I needed to use a ThreadLocal.

First I create a wrapper class for the context information called Message. The RpcHandler would then contain a private field of type ThreadLocal encapsulating a Message on a per-request basis. Even though ServletContainers like Jetty or Tomcat use a thread pool were Threads are recycled, this would still be okay as one Thread is used to service an entire request (actually this might be different in Servlet 3.0 where requests can be suspended and I'am assuming Threads are given back to the pool?). Quoting Brian Goetz:

ThreadLocal is also useful in servlet-based applications or any multithreaded server application in which the unit of work is an entire request, because then a single thread will be used during the entire course of handling the request.


Since usually each Spring Bean does only exist as 1 instance per ApplicationContext (unless specified otherwise), I decided to have a non-static ThreadLocal field which is sort of different from what the JavaDoc class comment suggests:

ThreadLocal instances are typically private static fields in classes that wish to associate state with a thread.


Here is an example of a typical RpcHandler subclass in the game:


public class SaveChangesHandler extends AbstractRpcHandler {

@Override
public void readRequest(final DatatypeInput in) {
// extract Message from DatatypeInput
setMessage(...);
}

@Override
public void performOperation() throws DAOException, RpcFailureException {
final Message message = getMessage();
// use Message to update the User
userService.save(message.getUser(), ...);
}

@Override
public void writeResponse(final DatatypeOutput out) throws IOException {
final Message message = getMessage();
// use Message to write something back to repsonse
out.write(message.getUser(), ...)
}

}


The AbstractRpcHandler class looks like this:


public abstract class AbstractRpcHandler extends RpcHandler<GameDaoSupport> {

private ThreadLocal<Message> threadLocalMessage;

public Message getMessage() {
return threadLocalMessage.get();
}

public void setMessage(final Message message) {
this.threadLocalMessage = new ThreadLocal<Message>();
this.threadLocalMessage.set(message);

// add a cleanup callback which will be
// executed from the ServletRequestCleanupFilter
ServletRequestCleanupService.addCleanupCallback(
new ServletRequestThreadLocalCleanupCallback(
this.threadLocalMessage
)
);
}
}


This worked well for some time but sometimes I could see the following errors in the logs:


2011-07-26 13:50:50 ERROR c.p.s.RpcServlet:handleRequest threw exception:
java.lang.NullPointerException: null
at com.foo.save.SaveChangesHandler.writeResponse(SaveChangesHandler.java:21)


Apparently the Message in writeResponse(..) was null, which was a bit strange because it was not null when performOperation(..) was invoked by the RpcServlet in stage 2. It took a bit but I found at least one problem with the code in AbstractRpcHandler:


public void setMessage(final Message message) {
this.threadLocalMessage = new ThreadLocal<Message>();
this.threadLocalMessage.set(message);

// add a cleanup callback which will be
// executed from the ServletRequestCleanupFilter
ServletRequestCleanupService.addCleanupCallback(
new ServletRequestThreadLocalCleanupCallback(
this.threadLocalMessage
)
);
}


The intention was to update the thread-local Message for the current Thread. Instead the code was reassigning the shared field to a new ThreadLocal instance and the previously assigned ThreadLocal instance was probably garbage collected. Here is a visualization of the problem flow:


Thread1 → setMessage(m1)
assigning new ThreadLocal1
ThreadLocal1.set(m1)
Thread1 → getMessage
ThreadLocal1.get → m1
Thread2 → setMessage(m2)
assigning new ThreadLocal2
ThreadLocal2.set(m2)
Thread1 → getMessage
ThreadLocal2.get → null *bang*


Took a while to spot the problem. I can really recommend to mark you static or non-static ThreadLocals as final, so this problem could not have occurred. I changed the AbstractRpcHandler to the following code and the errors disappeared.


public abstract class AbstractRpcHandler extends RpcHandler<GameDaoSupport> {

private final ThreadLocal<Message> threadLocalMessage = new ThreadLocal<Message>();

public Message getMessage() {
return threadLocalMessage.get();
}

public void setMessage(final Message message) {
this.threadLocalMessage.set(message);

// add a cleanup callback which will be
// executed from the ServletRequestCleanupFilter
ServletRequestCleanupService.addCleanupCallback(
new ServletRequestThreadLocalCleanupCallback(
this.threadLocalMessage
)
);
}
}


On a side note, I found it was a good practice to cleanup the ThreadLocals after the request was finished. Not really because this represented a memory problem as the number of Threads was limited by the Jetty thread pool. Rather I felt safer, if the recycled Thread serving a new Request would not see the Message of the previously handled HTTP Request. To do the cleanup using a Servlet Filter, I stole some code from the Apache Jetspeed 2 project (ServletRequestCleanupFilter, ServletRequestCleanupService, ServletRequestThreadLocalCleanupCallback and ServletRequestCleanupCallback).

1 Kommentare:

HLim hat gesagt…

Does cleaning up thread local not solve this problem?
so, you dont need to mark the thread local as final. not that I have any problem with marking it final, just curious.