70883 - Memory Leaks in LiveConnect

Status: VERIFIED FIXED

(Core Graveyard :: Java: Live Connect, defect)

Description

[Patrick C. Beard]

We leak every single JavaMemberDescriptor ever allocated in LiveConnect. This is
because destroy_java_member_descriptor() doesn't call JS_free() on
member_descriptor no member_descriptor->name. In addition, fur left the code in
jsj_DiscardJavaObjReflections() #if 0'd out because of an assertion problem. The
enclosed patch fixes the JavaMemberDescriptor leak (Boehm reports no leaks after
this patch is applied) and includes a hopefully non-assertion generating
implementation of jsj_DiscardJavaObjReflections().

Comment 1

[Patrick C. Beard]

Attachment: Fix memory leaks in LiveConnect.

Comment 2

[Brendan Eich [:brendan]]

beard: what could make JS_GetPrivate assert-botch?  Must have been the assertion
about obj's class having the JSCLASS_HAS_PRIVATE flag, it seems to me.  Could
there still be a problem with private-less objects (say, of class Object) being
passed to this code?

Otherwise, looks great -- glad you found this one!  r/sr=[e-mail]
assuming we're not about to hit the JS_GetPrivate assertion in spades, and/or
you know why fur did, but we don't.

/be

Comment 3

[Patrick C. Beard]

The older code in jsj_DiscardJavaObjReflections passed the JNIEnv* as the void* 
arg parameter to enumerate_remove_java_obj(), rather than a JSJavaThreadState. 
The assertions would happen because enumerate_remove_java_obj() would pass NULL 
for the JSContext* parameter to JS_GetPrivate(). This must have been some early 
code, because the code in jsj_DiscardJavaClassReflections() takes the same 
approach of passing the JSJavaThreadState as the void* arg, so I copied this 
approach to avoid the asserts.

Comment 4

[Patrick C. Beard]

Fix checked in.

Comment 5

[Brendan Eich [:brendan]]

What, no r=?  I wink, cuz it's a good fix, but we need more eyes on this code
than yours and mine, eh?

I thought by assertion you meant the real thing.  Passing null for cx to
JS_GetPrivate will crash hard, but not botch any assertion.  I was afraid fur
was dealing with a case where an object whose class has no private data flowed
into the JS_GetPrivate call.  Guess not, or it doesn't happen any longer, or we
will find out!

/be

Comment 6

[Phil Schwartau]

Marking Verified -