Clarify my responsibilities when I use Threadlocal variables in my Servlet.

[glassfishrobot]

A typical Servlet container services requests from a pool of threads. The Servlet container controls the lifecycle of those threads.

This creates a problem when my servlet (or a library which my servlet relies on) uses Threadlocal variables. For Tomcat, it means that when the context is undeployed, it has to check the viability of the threads in the pool to see if my code has 'polluted' them with Threadlocals. For example, it gives a warning like the following:

SEVERE: The web application [] created a ThreadLocal with key of type [org.apache.xmlbeans.impl.store.CharUtil$1] (value [org.apache.xmlbeans.impl.store.CharUtil$1@2aace7a7]) and a value of type [java.lang.ref.SoftReference] (value [java.lang.ref.SoftReference@3d9c9ad4]) but failed to remove it when the web application was stopped. Threads are going to be renewed over time to try and avoid a probable memory leak. Dec 13, 2012 12:54:30 PM org.apache.catalina.loader.WebappClassLoader checkThreadLocalMapForLeaks

Is this really an exceptional situation though? Can't I expect that the container will do as Tomcat has done, without feeling the need to blame me for it? Even if I did have the opportunity to provide some kind of 'thread-will-no-longer-perform-work-for-this-webapp' hook, I wouldn't necessarily be able to clean up - sometimes it is 3rd-party libraries which use Threadlocal.

See the question here for more discussion if you like.

[glassfishrobot]

@glassfishrobot Commented
Reported by dtbullock

[glassfishrobot]

@glassfishrobot Commented
markt_asf said:
If an application uses a ThreadLocal within a request then the application should ideally clean up that ThreadLocal before the request completes as there is no guarantee that the application will ever see that thread again. However, libraries often use ThreadLocals to retain expensive to create objects across requests. This makes clean-up more difficult although libraries that do this should also clean-up after themselves.

I'd have no objection to the requirement that code that creates a ThreadLocal is responsible for cleaning up that ThreadLocal when it is no longer required being spelt out in the Servlet spec.

I would argue that many uses of a ThreadLocal would be better implemented in a container environment by the use of a pool of objects bound to the ServletContext.

[glassfishrobot]

@glassfishrobot Commented
dtbullock said:
As an app-author, I don't mind providing a handler with a signature like void thisThreadIsBeingRetired() to try and clean up as best I can. However, please note that:

1. yes, I'd probably instead bind object pools to the ServletContext ... in my code;
2. I'd only be trying to clean up after 3rd-party libraries;
3. those 3rd party libraries:
   1. might not provide a way for me to say "hey, clean up the thread"
   2. might be using ThreadLocal for purposes other than object-pooling

As such, putting the responsibility on me (even with a handler to make it possible at all), is a half-way measure.

I'd rather not have to do anything, anyhow.

In the ultra-big picture of things, why would a container permit worker-threads to visit multiple ServletContexts over time and allow ThreadLocals to be set, yet have no way to perform reliable clean-up when the association between the worker-thread and the ServletContext is broken?

[glassfishrobot]

@glassfishrobot Commented
This issue was imported from java.net JIRA SERVLET_SPEC-82

[glassfishrobot]

• Issue Imported From: https://github.com/javaee/servlet-spec/issues/82
• Original Issue Raised By:@glassfishrobot
• Original Issue Assigned To: @shingwaichan

[gregw]

The API already has https://github.com/eclipse-ee4j/servlet-api/blob/master/api/src/main/java/jakarta/servlet/ServletRequestListener.java, which allows an app to be called when a thread dispatched for a request is about to leave scope. I think this is sufficient for most purposes, even though it doesn't cover threads calling async callbacks (for which threadlocals are marginal).

I think that it would be sufficient to add a spec section explicitly about threading that would cover:

• Once dispatched, the same thread is used to call all filters and the target servlet.
• The same thread is also used to call all filters and the new target servlet from within the scope of a RequestDispatcher.forward or RequestDispatcher.include call.
• Does an An ERROR dispatch resulting from a thrown exception or call to sendError use the same thread????
• An ERROR dispatch resulting from a call to sendError may not be done from within the scope of the call to sendError
• ASYNC dispatches may use a different thread to that which called AsyncContext.dispatch and does not occur within the scope of the call to AsyncContext.dispatch
• Async IO callbacks may be on different threads to that originally dispatched to the filters/servlet.

[stuartwdouglas]

This all sounds reasonable. IMHO sendError should use the same thread.