108474 - PRESERVE_JSOBJECT_IDENTITY must be defined for compatibility

Status: RESOLVED INCOMPLETE

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

Description

[Timothy J. Wood]

Well, I just spent my Sunday tracking down a 'bug' in the LiveConnect 
library.  I feel silly since I've already tracked this bug down once in the 
LiveConnect implementation I was working on before switching to the 
Mozilla implementation.  I say 'bug' because it is really a misuse of 
LiveConnect by Adobe.

  The problem is that the SVG plugin from Adobe doesn't obey the 'internal' 
part of JSObject.internal.  They try to work around a Netscape bug (I don't 
know what versions or if it is fixed already or what) where objects created 
via LiveConnect don't have their __parent__ and __proto__ slots set up 
correctly.

  So, they reach in and grab the internal field and set these slots.  Really.

  I figured out a bunch of this and contacted the SVG folks at Adobe who 
confirmed that (a) this is what happens and (b) the plugin is already 
shipping so it's going to have to stay that while for a way.

  They didn't have a good way of determining at runtime whether the bug 
was fixed (or at least didn't have time to figure it out).

  So in the Mozilla implementation of LiveConnect, this bug manifests 
itself as overwriting the pointer to the first JSArena in the JSArenaPool 
gcArenaPool in the JSRuntime.  This is because we have:

struct JSObjectHandle {
    JSObject *js_obj;
    JSRuntime *rt;
};

  but they are assuming that the internal field is a JSObject, so they read 
the second word of the internal pointer (the rt pointer here, the 'slots' in 
JSObject) and write to the first two words pointed to by that (the __proto__ 
and __parent__ fields in JSObject's slots).

  Sadly, when I try to build LiveConnect with 
PRESERVE_JSOBJECT_IDENTITY defined, I get a #error saying that it 
isn't fully implemented.  It really needs to and probably needs to be made 
the default (at least it does if you ever want to load the Carbon SVG plugin 
on Mac OS X).

  I'll be looking at what needs to be implemented still with this option, but I 
thought I would bring this to your attention.

  I'm not really sure why the option is still there if no one knew about this 
SVG grossness, but I didn't find any bugs mentioning 
PRESERVE_JSOBJECT_IDENTITY.


  Here is the mail from the SVG plugin developer talking about this issue:


Date: Thu, 18 Oct 2001 14:53:24 -0700
Subject: Mac OS X SVG plugin in OmniWeb
From: "Timothy J. Wood" <[e-mail]>
To: [e-mail]
X-Mailer: Apple Mail (2.472)



  Hi Paton!

  I was given your email address by Greg Christopher as a contact for the 
SVG plugin.

  I'm currently trying to get the SVG plugin fully functional in OmniWeb, 
including LiveConnect support.  With a small tweak I got OmniWeb to load 
the plugin and display SVG (basically to the same extent that IE does).  
I've got a chunk of LiveConnect  working now, but there is still quite a bit 
left to do.

  I've run into one pretty ugly stumbling block in that the SVG plugin reads 
the 'internal' field of my netscape.javascript.JSObject implementation 
(which is bad enough) but even worse, it reads and writes values through 
that pointer.  As the SVG plugin has no idea how my data is laid out, it 
clobbers memory by doing this.

  It ends up that the SVG plugin reads 'internal', then reads the second 
word of the returned value (as a pointer).  Finally the first words pointed to 
by this pointer is written.  It may be that the is even more access to this 
internal field later on in the program -- I've only looked at the immediate 
crasher problems so far in order to continued work on supporting the 
plugin.

  I have worked around this by setting up a big block of memory as my 
internal field and putting all my stuff way at the end (and setting up the 
second word to point the third to have a valid pointer above).

  We would really like to get the SVG plugin fully functional in some 
browser on the Mac (apparently IE doesn't support LiveConnect yet) and 
we certainly would hate to ship our LiveConnect support with this terrible 
hack.

  I presume that the SVG plugin is assuming some structure 
(undocumented as far as I can tell) on the part of the 'internal' field.  Our 
best outcome would be to have the plugin not access this field at all (hey, 
it is named 'internal' :), but failing that, if you could point us at some 
documentation about the structure of the internal field, we may be able to 
emulate it for the short term.


From: Peter Sorotokin <[e-mail]>
Date: Thu Oct 18, 2001  08:50:43 PM US/Pacific
To: [e-mail]
Cc: "Paton J. Lewis" <[e-mail]>
Subject: Re: Fwd: Mac OS X SVG plugin in OmniWeb

Tim,

It's amazing how you've been able to figure all this out! Your description is 
actually almost
complete.

SVG plug-in assumes that internal field references JSObject (C structure, 
not Java class,
referer to JavaScript interpreter source code that can be found in Mozilla 
distribution, see file jsapi.h).
Here is relevant comment from our source code (from the place where 
these assignemnts happen):

        // This is a NN JavaScript hack. We cannot create proper JSObject
        // through LiveConnect, unless call originated from JavaScript itself
        // JSObject.exec is not enough. But we cannot cause external 
JavaScript
        // to call us (the only way is to use javascript: URL, but that
        // causes random crashes). So we get handicapped object that 
LiveConnect
        // created and "fix" it by changing its __proto__ and __parent__
        // internal properties. NN JavaScript is garbage-collection based, so
        // no need to care about refcounts.

If we don't do this, we get not fully initialized javascript object from 
LiveConnect that
we cannot use as global object for our scripts. Therefore we must do this 
hack
for Netscape. This is the only place on Mac where we do think kind of 
hacks in
LiveConnect, so if you worked around those, you should be OK.

I can see this creating problems to you if your internal field does not 
exactly match
Netscape's... We will have to discuss what we can do about it, but since 
we are
long past RC, I cannot promise anything, even if it is trivial to do. Would 
sniffing your
"user agent" and not applying this "fix" if we see OmniWeb there be 
enough?

Note that we create our own "global" object and evaluate all our scripts in 
context
of this object. Unless parent link of this object is properly set to HTML 
window
object some scripts will not work properly in SVG plug-in. Netscape does 
not
do it properly (I think it is a bug in their code) and we "fix" it, so your object
must be correct or SVG scripting will not work right.

Peter

Comment 1

[Timothy J. Wood]

I've got most of a patch working but there is one ugly problem (which is 
presumably why the object handle goo was added in the first place).

In Java_netscape_javascript_JSObject_finalize we need to remove the 
root to the JavaScript JSObject (by calling 
jsj_remove_js_obj_reflection_from_hashtable).  This function currently 
requires a JSContext argument which it just uses to call 
JS_RemoveRoot().  We could just as easily use a JSRuntime (as the 
object handle code does) and call JS_RemoveRootRT().

I would propose that rather than creating an object handle and using that 
as the internal field, that another private field, "jsRuntime", be added to 
netscape.javascript.JSObject.  Hopefully there aren't ever enough 
JSObject instances around that the space for the extra pointer will matter.

This will allow removing the root from the hash table w/o trying to figure 
out what JSContext to use (which is hard since there isn't necessarily any 
other JavaScript on the stack and thus the embedding application doesn't 
necessarily have an opportunity to maintain a current context stack).


Comments?

Comment 2

[Timothy J. Wood]

Another alternative, obviously, would be to take JSRuntime used when 
adding the JavaScript JSObject to the hashtable and stick it in the bucket.

It looks like the hash table can have a pair of functions to allocate/
deallocate buckets so we could just define a custom bucket type that has 
a JSRuntime pointer in it too.

This sounds better than the extra field on netscape.javascript.JSObject.

Comment 3

[Timothy J. Wood]

The following patch turns on PRESERVE_JSOBJECT_IDENTITY by default and implements all the parts that were missing before.

In particular, the identity hash table buckets now store a pointer to the runtime for the object so that the finalize Java method can be implemented.  Added a trivial jsj_UnwrapJSObjectWrapper().

Implemented Java_netscape_javascript_JSObject_equals() to consider to object reference equal if they are the same Java object (according to JNI IsSameObject()).  This is correct because of the invariant that there is a one to one mapping between JavaScript JSObjects and Java JSObjects when PRESERVE_JSOBJECT_IDENTITY is defined.

Comment 4

[Timothy J. Wood]

Attachment: Suggested patch

Comment 5

[Phil Schwartau]

Reassigning to Patrick for consideration of this patch - 

Comment 6

[Bob Clary [:bc] (inactive)]

-> default assignee for old netscape assigned bugs.

Comment 7

[Alfred Peng]

Since the PRESERVE_JSOBJECT_IDENTITY part isn't fully implemented, there need to be a patch to complete it. How can we verify the patch with SVG plugins?

"njJSObject_long_internal" seems to be also an internal part of Java Object. Do we need another way to read it instead touching the "internal"? I think it's fine for us to read it. But It's not correct to write it.

In the bug description, it said that there isn't a good way to "create proper JSObject through LiveConnect, unless call originated from JavaScript itself". I guess the idea is that "LiveConnect don't have their __parent__ and __proto__ slots set up correctly." Then the question is can we fix the problem in Liveconnect itself? I notice that there are two member functions in "struct JSObjectOps" named "setProto" and "setParent". They are NULL in the "JavaObject_ops" for Java Object. Can we implement these functions to set the __parent__ and __proto__ instead writing to the slots directly?

Any comments?

Comment 8

[Benjamin Smedberg]

Firefox code moved from custom Liveconnect code to the NPAPI/NPRuntime bridge a while back. Mass-closing the bugs in the liveconnect component which are likely invalid. If you believe that this bug is still relevant to modern versions of Firefox, please reopen it and move it the "Core" product, component "Plug-Ins".