Message5879

Here are a few comments on the changes I made. Hopefully this will shed some light on why I coded things the way I did.

The fundamental problem is that classes from the jython jar file are contained in thread-local storage. This is what causes the classloader leak when a servlet hosting the jython jar file is reloaded (because tomcat and most other app servers use thread pools, so the threads in the pools contain references to classes in the old classloader.)

What I would *love* to see is a move away from using TLS at all, but barring that, there are two ways to clean up thread local storage on shutdown: 
  (1) Iterate all the threads and access the private member variable that holds the thread locals. Sample code that does this can be found here: http://blog.igorminar.com/2009/03/identifying-threadlocal-memory-leaks-in.html and here: http://weblogs.java.net/blog/jjviana/archive/2010/06/09/dealing-glassfish-301-memory-leak-or-threadlocal-thread-pool-bad-ide . I rejected this approach because it (a) ties you to a specific JRE implementation and (b) requires the JVM be running with certain security setting to allow exposing private member variables.
  (2) Don't store our classes in TLS - this is the approach I took. Rather than store org.python.core.ThreadState objects directly in TLS, I store Object[1] instances. I put the ThreadState object into the first element in this array. I also keep track of all the arrays I have put in TLS using weak references in the array "threadStateList". When it comes time to cleanup, I simply iterate this list, nulling out the first element of each array. Thus, each thread still has a TLS variable, but it is an object array with null in it, so there are no references to any org.python.* classes.

The boolean "isShutdown" prevents stuff from getting added back to TLS after we have cleaned up. For example, I have found that the finalizer thread can add python thread locals (see my other post) 

Because I introduced some global state (threadStateList and isShutdown), I also had to add some synchronization to this class. However, I didn't want to make the usual case (get from TLS after it has been set) to be slower. That is why the initial check for TLS at the beginning of getThreadState() is not synchronized - the synchronized block comes after that check. In this way, most calls to this method should be as fast as they were before.

I think that should explain everything - let me know if there are any other questions/concerns.