Message5876

Author matt_brinkley
Recipients adam.spiers, amak, colinhevans, fwierzbicki, matt_brinkley, pjenvey, zyasoft
Date 2010-07-06.23:27:52
SpamBayes Score 2.649197e-07
Marked as misclassified No
Message-id <1278458874.56.0.228025853016.issue1327@psf.upfronthosting.co.za>
In-reply-to
Content
Jim: thanks for checking bug status while camping - that is impressive dedication! :)

Update: I found that my fix is not 100% sufficient - it turns out I cannot easily intercept all thread paths into the jython runtime - for example, I found the JVM the Finalizer thread, when calling finalize() on certain jython classes, can trigger thread-local storage to be created (e.g. in PyGenerator and PyFinalizableInstance) Also, I am afraid there could be other cases where python classes that subclass Java classes/interfaces are associated with other (pure) Java classes and thus jython code can be called without me having the ability to wrap it in a try/finally and clear out the thread-local storage.

So, I made a more substantial change to ThreadStateMapping that keeps track of everything added to TLS and includes a new "shutdown()" method that is meant to be called when the classloader is being destroyed (e.g. when tomcat is trying to reload a servlet). This code does indeed work, and I have verified that classloaders are garbage collected when I call this.

The code is a little less intuitive that my other changes - I can provide more rationale/explanation if need be.

------------------------------------------------------------------
ThreadStateMapping.java
------------------------------------------------------------------

package org.python.core;

import java.util.ArrayList;
import java.lang.ref.WeakReference;

class ThreadStateMapping {
    //private static final ThreadLocal<ThreadState> cachedThreadState = new ThreadLocal<ThreadState>();
    private static final ThreadLocal<Object[]> cachedThreadState =
            new ThreadLocal<Object[]>()
            {
                @Override
                protected Object[] initialValue()
                {
                    return new Object[1];
                }
            };

    private ArrayList<WeakReference<Object[]>> threadStateList = new ArrayList<WeakReference<Object[]>>();
    private boolean isShutdown = false;

    public ThreadState getThreadState(PySystemState newSystemState) {

        //initial check for usual case outside syncronized block for efficiency
        ThreadState ts = (ThreadState)cachedThreadState.get()[0];

        if (ts != null) {
            return ts;
        }

        synchronized(this)
        {
            //check again within synchronized block to guard against race condition
            Object[] threadLocal = cachedThreadState.get();
            if(threadLocal[0] != null)
                return (ThreadState)threadLocal[0];

            Thread t = Thread.currentThread();
            if (newSystemState == null) {
                Py.writeDebug("threadstate", "no current system state");
                if (Py.defaultSystemState == null) {
                    PySystemState.initialize();
                }
                newSystemState = Py.defaultSystemState;
            }

            ts = new ThreadState(t, newSystemState);

            if(! isShutdown)
            {
                threadLocal[0] = ts;
                threadStateList.add(new WeakReference<Object[]>(threadLocal));
            }
            return ts;
        }
    }


    public synchronized void shutdown()
    {
        isShutdown = true;
        for(WeakReference<Object[]> ref : threadStateList)
        {
            Object[] o = ref.get();
            if(o != null)
            {
                o[0] = null;
            }
        }
        threadStateList.clear();
    }
}

------------------------------------------------------------------
A slightly different method name in Py.java:
------------------------------------------------------------------
    public static void clearAllThreadLocalState()
    {
        threadStateMapping.shutdown();
    }
History
Date User Action Args
2010-07-06 23:27:54matt_brinkleysetmessageid: <1278458874.56.0.228025853016.issue1327@psf.upfronthosting.co.za>
2010-07-06 23:27:54matt_brinkleysetrecipients: + matt_brinkley, fwierzbicki, amak, pjenvey, zyasoft, colinhevans, adam.spiers
2010-07-06 23:27:54matt_brinkleylinkissue1327 messages
2010-07-06 23:27:52matt_brinkleycreate