494665 - Memory leak when using Java reflection & shared scopes

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Attila Szegedi]

From the e-mail Kieran Topping sent to the rhino mailing list: 

When using the default WrapFactory, ClassCache holds a reference to whatever the top level scope was at the time the class-in-question was cached.
Usually, this is truly the top-level scope, which doesn't cause any problems.
However, if you're using a shared "global" scope, as per:
https://developer.mozilla.org/En/Rhino_documentation/Scopes_and_Contexts#Sharing_Scopes
then this sharedScope isn't used by ClassCache; it's actually your "newScope", as follows:

Scriptable sharedScope = ... create shared scope.

create a scope that uses the shared scope
  Scriptable newScope = cx.newObject(sharedScope);
  newScope.setPrototype(sharedScope);
  newScope.setParentScope(null);
then run a script in newScope, in particular one that ends up wrapping a java class, either via Context.javaToJS() or actually in your javascript. Even if you don't do this explicitly, things like throwing a java exception will end in WrapFactory being used.

After the above, you can end up with the following:

sharedScope
-stores-reference-to-> classCacheInstance

classCacheInstance
-stores-reference-to-> newScope.

This means that newScope will /never /become eligible for garbage collection. If you created some large objects in newScope then these will never be GC-ed either. Hence....memory leak.

Comment 1

[Attila Szegedi]

Attachment: Patch that fixes the bug

Attached a patch to fix the bug. Basically, have the JavaMembers objects bind to the scope of the ClassCache instead of the current top-level scope (as the ClassCache scope can be the current top-level scope's prototype).

Comment 2

[Attila Szegedi]

Committed patch to CVS HEAD:

***
cvs ci -m "Fix for #494665: Memory leak when using Java reflection & shared scopes" -l "/Rhino/src/org/mozilla/javascript/JavaMembers.java" "/Rhino/src/org/mozilla/javascript/ClassCache.java"
    Checking in src/org/mozilla/javascript/ClassCache.java;
    /cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/ClassCache.java,v  <--  ClassCache.java
    new revision: 1.20; previous revision: 1.19
    done
    Checking in src/org/mozilla/javascript/JavaMembers.java;
    /cvsroot/mozilla/js/rhino/src/org/mozilla/javascript/JavaMembers.java,v  <--  JavaMembers.java
    new revision: 1.80; previous revision: 1.79
    done
ok (took 0:04.203)
***

Comment 3

[Hannes Wallnoefer]

Comment on attachment 379424 [details] [diff] [review]
Patch that fixes the bug

Review of attachment 379424 [details] [diff] [review]:
-----------------------------------------------------------------

I haven't seen this at the time, and bugzilla has recently started sending me me reminder emails about it, so yes, this looks perfectly fine :)