Issue2597

I rechecked the problem encountered in an earlier Jython version (2.5.2) which was a memory leak described in this bug report: http://bugs.jython.org/issue2026 . The bug is marked as fixed and I wanted to double check this. Therefore, I used the code snipped contained in the bug report to reconstruct the issue. The original issue seems to be resolved, but I encountered another problem. If I run the aforementioned code for a short period of time, the consumed heap increases quite fast: ~120MB after ~60 seconds. I inspected the heap dump and most the memory is consumed by a ConcurrentHashMap in PySystemState. I took a quick peek at the source and found this:

PySystemState has a static final ConcurrentHashMap named sysClosers [0]. If a new PySystemStateCloser is created, a Key-Value-Pair of a WeakReference to a PySystemState and a reference to the PySystemStateCloser is added to the map [1], but never removed. Even if the WeakReference is finalized, if looks like the value will still remain in the map, since there is no removal code for broken references.

Given that we bundle Guava anyway, maybe the solution is as easy as changing sysClosers to be

new com.google.common.collect.MapMaker().weakKeys().makeMap()

In the doc they don't explicitly specify that such a map would automatically tidy up cleared keys, but they advertise it as a sutiable replacement for java.util.WeakHashMap. WeakHashMap doc explicitly states that it is self-cleaning, so I suppose this implies the Guava-version s either.

I will give it a try if I can reproduce this issue (didn't check out the code yet).

I ran the `Exhaust` test from #2026 for 250000+ iterations, and under inspection with VisualVM, I did not see any leak.

Perhaps we can optimize here, but it seems to me that the problem reported here is not a memory leak per se, as was in the original issue, but using up memory too quickly/inefficiently. Changing the semantics here can be subtle, as we saw with somewhat related #2321.

I tested this using the attached program mem_test.java.
Note that in this form it requires a minimally modified Jython-build. For simplicity of testing this I changed PySystemState.sysClosers to be public.
Indeed it turns out that PySystemState.sysClosers.size() is constantly growing, about 40 per 500 iterations. After 20000 iterations I have 1541 sysClosers.
So, even if this might not be much notable in memory load, I'd call it some kind of leak. As next step I will check if the proposed solution cures this...

Fixed as of https://hg.python.org/jython/rev/a3b75e136195.
I used an easier approach; just added a remove-command to the cleanup method.

@tobias: Would be good if you can confirm this fixes it.

Running the test mentioned above with this fix keeps sysClosers.size() constantly at a sane level varying between 25 and 45 over tenthousands of iterations.
I did not encounter additional regrtest failures so far.

Looks like the right change to me - Jython needs to discard this tracking data upon doing the closing. I'm going to modify the title and corresponding NEWS entry however, because it potentially suggests this problem occurred in RC2, whereas this has been since we introduced the current reference queue approach for closers.

I just retested this with the fix included and can confirm that the memory foodprint is way better than before.