213279 - Scope should not be cached through JavaMembers.classTable

Status: RESOLVED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Igor Bukanov]

Rhino uses JavaMembers.classTable which is a static Hashtable to cache
reflection information about Java classes. Currently the cache stores not only
Method and Field instances for a particular class, but also top level scope of
the first script that initializes a cache entry for a particular class. It not
only prevents scope objects from garbage collection but also allows to access
objects from this scope from different runtime invocation. 

The following attached Java source demonstrate this incorrect exposure by
executing 2 times

java.lang.System.out.println.__proto__ === Function.prototype

in 2 different scopes fully initialized with their own instances of the runtime
library objects.

The first execution produces the expected true result while the second gives
false since cached JS wrapper for java.lang.System.out.println still points with
its prototype to Function.prototype from the first scope.

Comment 1

[Igor Bukanov]

Attachment: Test case as Java source

To run the test, compile the Test.java and run it. Currently it produces:

~/tmp> java Test
Executing: java.lang.System.out.println.__proto__ === Function.prototype
First execution result:  true
Second execution result: false

while the expected resultis is true for both executions.

Comment 2

[Igor Bukanov]

Attachment: Preliminary work: separation of reflection data and corresponding JavaScript wrappers.

The patch changes JavaMemebers to separate between Java fields and methods with
the same name and corresponding JavaScript wrappers.

I added MemberFamily class to hold all members with the same name including
synthetic bean properties. In addition the class contains sharedWrapper field
to hold a JS wrapper for the given Java name that does not require
corresponding Java object.  Internal hash tables in JavaMemebers now always
contain instances of this class as values instead of mixed bag of
NativeJavaMethod, BeanInfo and Field instances.

The other notable change is that FieldAndMethods that is used to wrap Java
field and methods with the same name now uses delegation to call
NativeJavaMethod in () or new context instead of extending from
NativeJavaMethod. In this way a single shared NativeJavaMethod will be used to
represent method part of all FieldAndMethods objects. In addition instances of
FieldAndMethods corresponding to the same name static field and methods is also
shared since neither field nor methods need java object in this case.

Now the bug emphasis itself in presence of MemberFamily.sharedWrapper with
explicit scope references that is stored in a static cache of JavaMemberse and
a fix would require to store sharedWrapper in scope instead.

Comment 3

[Igor Bukanov]

Attachment: Move NativeJavaPackage.TopLevelPackage into a separated source file NativeTopJavaPackage

Another preliminary patch. It moves internal NativeJavaPackage.TopLevelPackage
class into a separated source file under the new name NativeTopJavaPackage. The
idea is to store a class cache there and pass it around to other classes that
uses JavaMembers.

Comment 4

[Igor Bukanov]

Attachment: Small fixes for the previous patch

Comment 5

[Igor Bukanov]

I committed the previous patch.

Comment 6

[Igor Bukanov]

Attachment: Fix: store caches inside global scope

The patch adds new class, omj.GlobalScope that is supposed to serve as a global
scope for the standard library and which stores previously static caches. 

The class contains a static method get(Scriptable) to extract a class instance
from a scope object. JavaMembers and FunctionObject is updated to use this
method to access the caches. The method searches the prototype chain for
instance of GlobalScope and if it found, it is returned. If not, then a
property __globalScope is queried for the class instance. If that does work, a
new allocated object is returned so  GlobalScope.get() will never return null.

Context.initStandardObjects will create __globalScope property in the scope
object when explicitly passed scope is not an instance of GlobalScope. Another
way to address compatibility would be to point explicit scope prototype to a
new allocated GlobalScope instance, but it may bring more issues if an
application assumes a particular structure of its global scope prototype chain.

Comment 7

[Igor Bukanov]

Attachment: Previous fix with indentation fixes

Comment 8

[Igor Bukanov]

I committed the fix.

Comment 9

[Igor Bukanov]

Trageting as resolved against 1.5R5