57962 - xpconnect apparently confused by recycled JSContext pointers

Status: VERIFIED FIXED

(Core :: XPConnect, defect, P3)

Description

[John Bandhauer]

I'm writing this bug for the benefit of those w/o bugscape access since I want 
to check this into the mozilla tree....

http://bugscape.netscape.com/show_bug.cgi?id=2952

There is a bugscape bug about the browser not handling the the "%ip" JS argument 
converter on the whole run when the activation dialog is the first window up.

This is seen on Win2k only.

I spoke with racham about this and decided that this is probably a recycled 
pointer problem. I'll paste some text from that bug and attach the patch. I'd 
like some review/approval for getting this into the trunk to bolster the 
argument for going into rtm.

Thanks.

...............................................................................

Some rationale behind this patch...

XPConnect maintains a list of objects of class XPCContext that are one-to-one 
with the JS engine's JSContext objects. XPConnect does not receive direct 
notifications when JSContexts are created and destroyed (and thus the XPCContext 
list becomes out of sync with the list of live JSContexts). This is not 
a problem. Instead, XPConnect synchronizes its list whenever it 'sees' a 
JSContext it has not seen before or when a call is made to 
nsIXPConnect::SyncJSContexts(). The DOM makes that call after destroying a 
JSContext.

The only reason I could come up with for the observed behavior is based on the 
assumption that the heap manager in the C rumtime shipped with Win2K is more 
aggressively recyclying fixed sized objects such that a malloc of 
sizeof(JSContext) is more likely to receive a recently free'd block of 
sizeof(JSContext). So, I think that a JSContext is being destroyed and a new one 
created at the same address in the heap. If XPConnect does not re-sync its 
XPCContext list inbetween the destruction and creation then it will have no 
reason to suspect that the JSContext at the given address is any different than 
the one previously at that address. So... It will not re-register the JS 
argument formatter for "%ip" for that JSContext.

nsJSContext (yet another one-to-one with JSContext C++ class in the DOM) was 
already calling nsIXPConnect::SyncJSContexts() in its destructor after it called 
JS_DestoryContext. I suspect that some other system (maybe prefs) was creating 
and destroying a JSContext whose pointer is later recycled.

My fix is to call nsIXPConnect::SyncJSContexts() in the nsJSContext constructor 
before it calls JS_NewContext (in addition to the existing call in the 
destructor). This way even if the pointer is recycled, XPConnect will have 
removed its old representation of the JSContext and thus will correctly 'notice' 
the new one.

I can see no reason why this fix would break anything.

Comment 1

[John Bandhauer]

Attachment: proposed fix from bugscape bug

Comment 2

[Mike McCabe]

r=mccabe.

Comment 3

[Brendan Eich [:brendan]]

Cite this bug instead of the bugscape bug, and sr=[e-mail].

/be

Comment 4

[lchiang]

I'll mark this bug rtm+ for review and I'll go close the bugscape bug which I'll
consider a dupe of this one.

Comment 5

[Jim Roskind]

marking rtm double plus.  Please land on N6 branch asap.
This is a dupe of a Netscape stop-ship bug seen in bugscape.

Comment 6

[John Bandhauer]

Patch checked in to both the trunk and Netscape_20000922_BRANCH.

racham, since you know how to reproduce the original bug, would you please 
verify that the checkin has fixed the problem (or arrange for someone else to 
do so).

Thanks, John.

Comment 7

[racham]

Didn't see any problems (problems like alerts not working, forms not getting
submitted) originally reported in bugscape bug 2952. Things work fine.

John Morrison, you originally reported the bugscape bug 2952. Please make sure
that you repeat those steps and verify the bug accordingly. All my tests went
OK. Please test on your end and mark it verified when you are done. Also, some
testing from Terri Preston (reported bugzilla bug 55566) will be great.

Comment 8

[John Morrison]

Going through the same procedure to reproduce from bugscape 2952, on either a 
migrated or newly-created profile (win2k), I don't have any problems: 
javascript alert, confirm and prompt work, http basic-authentication works, 
http post form submission works, the windows desktop integration dialog comes 
up. This is fixed 2000102613 MN6 branch and 2000101608 MTrunk win2k. Marking 
VERIFIED. Thanks, jband.