Issue2127

Hi, I've started using Jython (from inside Java) and discovered a problem; when a weakref is imported (eg. by 'import logging') objects are not being recycled 

Please look at the attached unittest to reproduce the problem - I'm using jython2.7.b1

You will need to use http://labs.carrotsearch.com/randomizedtesting.html

This is what I'm seeing:


Apr 20, 2014 1:54:17 AM com.carrotsearch.randomizedtesting.ThreadLeakControl checkThreadLeaks
WARNING: Will linger awaiting termination of 1 leaked thread(s).
Apr 20, 2014 1:54:37 AM com.carrotsearch.randomizedtesting.ThreadLeakControl checkThreadLeaks
SEVERE: 1 thread leaked from SUITE scope at org.jython.monty.TestJythonWeakref: 
   1) Thread[id=13, name=weakref reaper, state=WAITING, group=TGRP-TestJythonWeakref]
        at java.lang.Object.wait(Native Method)
        at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:135)
        at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:151)
        at org.python.modules._weakref.GlobalRef$RefReaperThread.collect(GlobalRef.java:212)
        at org.python.modules._weakref.GlobalRef$RefReaperThread.run(GlobalRef.java:221)
Apr 20, 2014 1:54:37 AM com.carrotsearch.randomizedtesting.ThreadLeakControl tryToInterruptAll
INFO: Starting to interrupt leaked threads:
   1) Thread[id=13, name=weakref reaper, state=WAITING, group=TGRP-TestJythonWeakref]
Apr 20, 2014 1:54:40 AM com.carrotsearch.randomizedtesting.ThreadLeakControl tryToInterruptAll
SEVERE: There are still zombie threads that couldn't be terminated:
   1) Thread[id=13, name=weakref reaper, state=WAITING, group=TGRP-TestJythonWeakref]
        at java.lang.Object.wait(Native Method)
        at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:135)
        at java.lang.ref.ReferenceQueue.remove(ReferenceQueue.java:151)
        at org.python.modules._weakref.GlobalRef$RefReaperThread.collect(GlobalRef.java:212)
        at org.python.modules._weakref.GlobalRef$RefReaperThread.run(GlobalRef.java:221)

From what I can reason from the code and the test case the issue is that the RefReaperThread is leaked rather than objects in the ReferenceQueue not being GC'd.  

The attached patch *should* allow the reference reaper thread created by GlobalRef to die *and* respawn if necessary. It does this be changing the RefReaper to a Runnable (attempting to avoid the issue detailed in https://bitbucket.org/jython/jython/pull-request/19/pysystemstateclosershutdowncloser-use/diff) and implementing Callable so that it can be registered with the PySystemState as a resource closer and thus be cleaned up when PySystemState.cleanup() is called.

I think I managed to run rchyla's test case successfully once I added a call to the interpeter's cleanup method, i.e.:
            interp.exec("import weakref");
            interp.cleanup();


I'm not sure of the best way to incorporate a test for this into the build so any pointers as to how/where to add a test would be great.

Target beta 4

Applied Indra's patch in https://bitbucket.org/jimbaker/jython-resource-leaks, with the intent to merge this into trunk sometime soon, maybe this week

Note that there are a number of failing tests. I don't think we need to support recursion_limit, that's clearly a CPython-implementation detail. It's interesting to see that the change in ThreadStateMapping to use a weakkey/weakval map instead of a ThreadLocal is enough to cause a failure in test_weakset. However, I really doubt that _IterationGuard is as threadsafe as it claims, at least as it interacts with the remove method, given that 

                if self._iterating:
                    self._pending_removals.append(item)
                else:
                    self.data.discard(item)

is clearly not guaranteed to be atomic. The solution is to revisit this implementation with the use of Guava MapMaker.

Re incorporating Roman's test - it does look good, with the call to cleanup (soon to be a synonym for close). I especially like the idea of using com.carrotsearch.randomizedtesting.annotations to check for thread leaks. Just a matter of figuring out the necessary changes to our build.xml and placement into our existing JUnit tests

Fixed as of http://hg.python.org/jython/rev/4e538d0ebbd7