Fix memory leak when used from multiple JRuby runtimes.

[jfirebaugh]

If json/ext/parser is required from multiple JRuby runtimes in the same JVM, it will leak memory for every runtime beyond the first.

Test script: https://gist.github.com/971319

The leak is caused by taking references to the runtimes in the RuntimeInfo class, particularly the WeakHashMap<Ruby, RuntimeInfo>. Note the caveat in the WeakHashMap JavaDocs:

Implementation note: The value objects in a WeakHashMap are held by ordinary strong references. Thus care should be taken to ensure that value objects do not strongly refer to their own keys, either directly or indirectly, since that will prevent the keys from being discarded. Note that a value object may refer indirectly to its key via the WeakHashMap itself; that is, a value object may strongly refer to some other key object whose associated value object, in turn, strongly refers to the key of the first value object. One way to deal with this is to wrap values themselves within WeakReferences before inserting, as in: m.put(key, new WeakReference(value)), and then unwrapping upon each get.

This applies here, because each RuntimeInfo has internal references to the corresponding Ruby object via the members jsonModule, stringExtendModule, etc -- these will all contain internal references to the runtime.

Using WeakReference as the value object instead fixes the leak as shown by the test script.

Note: also reported here. It looks like active development for all platforms is going on in this repository now, so I submitted the pull request here. Let me know if this is not the desired workflow.

[jfirebaugh]

I'm having second thoughts about this patch. I suspect it will let the RuntimeInfo get garbage collected as soon as no local references to it exist, which isn't what we want. What want is for the runtime to have a single reference to the associated RuntimeInfo, and no other references to exist.

A similar problem exists in JRuby's FFI library: http://jira.codehaus.org/browse/JRUBY-5053

[jfirebaugh]

I can confirm that this patch is no good as written; it causes crashes when using the library from a second runtime.

@headius, can you help us out here?

Here's the background: the JSON JRuby implementation has a RuntimeInfo Java class whose main purpose is to provide a per-runtime cache of frequently used objects (the RubyModule for JSON extension, the RubyEncoding for UTF8 and ASCII-8-Bit, etc.) so as to avoid repeatedly looking them up (via getConstant, RubyEncoding.find, etc.). It currently keeps track of the runtime-to-RuntimeInfo mapping using a WeakHashMap, which causes memory leaks because the RuntimeInfo values indirectly reference their associated Ruby keys, and so neither the values or keys will ever be garbage collected.

My attempted fix was to use a WeakReference<RuntimeInfo> as the value, but as nothing keeps a hard reference to the RuntimeInfo, this causes it to be prematurely garbage collected. What want is for each runtime to have a single reference to the associated RuntimeInfo, and no other references to exist. But I'm not sure how to accomplish that.

[mernen]

A simplified schema of the reference chain today can be seen here: http://i.imgur.com/CA0Mf.png
The problem is that a strong reference chain is always kept to Ruby runtimes, preventing them from being garbage collected.

The current patch produces this chain: http://i.imgur.com/mzNek.png
Now no strong references are kept to RuntimeInfo objects, leaving them for garbage collection.

I believe the solution then is to keep a strong reference to RuntimeInfo, but turn all its member variables that point to runtime objects into weak references, like this: http://i.imgur.com/zTgUR.png

Could you try this approach?

[jfirebaugh]

Great diagrams @mernen.

I'm working on a patch along the lines you suggest.

[jfirebaugh]

Updated patch.

[mernen]

Just a note: for encodingsSupported, I think the test for utf8 != null should suffice. We aren't (or shouldn't) ever set it to a new WeakReference(null), and we're just using the existence of that value as a cheap way of checking if the runtime supports encodings. The only way utf8.get() could be null would be if the runtime got rid of its reference, a completely unexpected situation. Even then, if somehow the encoding object were garbage collected, that wouldn't change the runtime's nature as supporting Ruby 1.9 encodings.