JRuby

ThreadLocal RubyEncoding.UTF8_CODER leaks class loader

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: JRuby 1.6.6
  • Fix Version/s: JRuby 1.6.8
  • Component/s: Encoding, Rails WAR Deployment
  • Labels:
    None
  • Environment:
    Linux w/ OpenJDK 1.6.0_23
  • Number of attachments :
    1

Description

If one loads JRuby from a new classloader (different from that of the current thread), instantiates a Ruby instance, then tearDown and nullifies the Ruby instance, the JRuby classes loaded by the new classloader are not freed because an object that extends ThreadLocal remains and references that classloader. At least one such ThreadLocal appears to be at org/jruby/RubyEncoding.java:265 stored in UTF8_CODER.

I've detailed sample code that both shows the leak and works around it: http://stackoverflow.com/questions/9541207/loading-jruby-at-runtime-and-classloader-leak

A rough outline is:

In a loop in a single thread:
load JRuby from a new classloader
instantiate Ruby from that classloader
tear it down
PermGen space OutOfMemoryException after several (~13) iterations

Work around:

In a loop in a single thread:
load JRuby from a new classloader
instantiate Ruby from that classloader in a separate thread
tear it down
stop that thread
100 iterations complete with no problems

I see there are other ThreadLocal bugs reported here, some of relevance to running with JRuby-Rack and/or servlet containers. Not sure if this is fixable or if it just must be lived with. Is being redeployable in a servlet container currently possible and/or a JRuby goal?

Some relevant references: http://wiki.apache.org/tomcat/MemoryLeakProtection
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6254531

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Charles Oliver Nutter added a comment -

Drat...you are absolutely right. This needs to be fixed, perhaps with a weak/soft reference or by some other mechanism.

Show
Charles Oliver Nutter added a comment - Drat...you are absolutely right. This needs to be fixed, perhaps with a weak/soft reference or by some other mechanism.
Hide
Permalink
Patrick Mahoney added a comment -

Here's a possible fix using a WeakReference. I'm not sure this is appropriate, since in some cases it may be no different than just calling new UTF8Coder() each time.

Even with this patch in place, my original test case still fails (classloaders remain referenced by finalizer in org.jruby.util.io.ChannelStream), but I don't see any references from RubyEncoding$2.

Show
Patrick Mahoney added a comment - Here's a possible fix using a WeakReference. I'm not sure this is appropriate, since in some cases it may be no different than just calling new UTF8Coder() each time. Even with this patch in place, my original test case still fails (classloaders remain referenced by finalizer in org.jruby.util.io.ChannelStream), but I don't see any references from RubyEncoding$2.
Hide
Permalink
Charles Oliver Nutter added a comment -

I have made the fix on master@929056a and jruby-1_6@f3b846a. I opted to use a SoftReference, since a WeakReference would usually get scrubbed too quickly. Soft references will age out over time or be cleaned en masse when there's memory pressure.

This is a tricky one for us to test. Can you confirm that it's looking better for you?

Show
Charles Oliver Nutter added a comment - I have made the fix on master@929056a and jruby-1_6@f3b846a. I opted to use a SoftReference, since a WeakReference would usually get scrubbed too quickly. Soft references will age out over time or be cleaned en masse when there's memory pressure. This is a tricky one for us to test. Can you confirm that it's looking better for you?
Hide
Permalink
Patrick Mahoney added a comment -

My test ran 100 iterations with no OutOfMemory, so looks good.

Note that I cherry-picked your patch against the 1.6.7 tag since the master branch is throwing exceptions for me (this has been the case for all my work on this issue; probably irrelevant).

(Strange that in my previous comment (with WeakReference version) I was still filling up PermGen... not really sure what happened there; it's possible I applied my patch to 1.6.6, but I've no idea if there are relevant differences to ChannelStream between 1.6.6 adn 1.6.7).

Anyway, thanks for the quick response on this issue!

Show
Patrick Mahoney added a comment - My test ran 100 iterations with no OutOfMemory, so looks good. Note that I cherry-picked your patch against the 1.6.7 tag since the master branch is throwing exceptions for me (this has been the case for all my work on this issue; probably irrelevant). (Strange that in my previous comment (with WeakReference version) I was still filling up PermGen... not really sure what happened there; it's possible I applied my patch to 1.6.6, but I've no idea if there are relevant differences to ChannelStream between 1.6.6 adn 1.6.7). Anyway, thanks for the quick response on this issue!
Hide
Permalink
Charles Oliver Nutter added a comment -

Well we'll call it fixed and I'll do an audit for other ThreadLocal use in JRuby.

Show
Charles Oliver Nutter added a comment - Well we'll call it fixed and I'll do an audit for other ThreadLocal use in JRuby.
Hide
Permalink
Charles Oliver Nutter added a comment -

Filed JRUBY-6523 for additional cases we should look into.

Show
Charles Oliver Nutter added a comment - Filed JRUBY-6523 for additional cases we should look into.
Hide
Permalink
Patrick Mahoney added a comment -

A comment on a blog post suggests that using SoftReference will still leak the reference itself. This is orders of magnitude better than leaking the ClassLoader, of course.

Soft references do not fully solve the problem

Soft references do not fully solve the problem. When you store a SoftReference<T> in ThreadLocal, the T may go away after a while, but the SoftReference itself remains until you call ThreadLocal.remove(). If T is a class from your web app, this may help to avoid a classloader leak, but the (expired) SoftReferences still accumulate in ThreadLocal.

Tomcat, after redeploying a webapp, does renew the threads in its thread pool; over time each each thread in the pool is removed and replaced with a newly created thread. Thus dangling ThreadLocals will be garbage collected (this is true for SoftReference or JRuby object ThreadLocals).

I don't think there is further work to do (eliminating ThreadLocals without hurting performance seems like a task with much larger scope), I'm just adding this comment as documentation.

Show
Patrick Mahoney added a comment - A comment on a blog post suggests that using SoftReference will still leak the reference itself. This is orders of magnitude better than leaking the ClassLoader, of course. Soft references do not fully solve the problem Soft references do not fully solve the problem. When you store a SoftReference<T> in ThreadLocal, the T may go away after a while, but the SoftReference itself remains until you call ThreadLocal.remove(). If T is a class from your web app, this may help to avoid a classloader leak, but the (expired) SoftReferences still accumulate in ThreadLocal. Tomcat, after redeploying a webapp, does renew the threads in its thread pool ; over time each each thread in the pool is removed and replaced with a newly created thread. Thus dangling ThreadLocals will be garbage collected (this is true for SoftReference or JRuby object ThreadLocals). I don't think there is further work to do (eliminating ThreadLocals without hurting performance seems like a task with much larger scope), I'm just adding this comment as documentation.
Hide
Permalink
Charles Oliver Nutter added a comment -

Thanks for the update. I knew about this minor "leak" but figured it was acceptable, since SoftReference is pretty small and we're only attaching a couple to each thread.

Also good to know that Tomcat does periodically refresh its threads. Thanks!

Show
Charles Oliver Nutter added a comment - Thanks for the update. I knew about this minor "leak" but figured it was acceptable, since SoftReference is pretty small and we're only attaching a couple to each thread. Also good to know that Tomcat does periodically refresh its threads. Thanks!

People

  • Assignee:
    Charles Oliver Nutter
    Reporter:
    Patrick Mahoney
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: