496486 - Crash while running style system mochitest [@ nsXPConnect::ReparentScopeAwareWrappers]

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Boris Zbarsky [:bzbarsky]]

One of the Mac m-c mochitest runs today crashed while running layout/style/test/test_parse_rule.html.  The output is:

58166 INFO TEST-PASS | /tests/layout/style/test/test_parse_rule.html | #a {border-style:solid}#a {border-width: 50zu}
<a name='err0'></a><a href='#err1'>NEXT ERROR</a> <font color='000080'>TEST-UNEXPECTED-FAIL | automation.py | Exited with code 1 during test run

That test-pass mentioned there is partway through the test, so the crash was in fact on this test, not the next one.  The stack:

 0  XUL!nsCOMPtr_base::assign_from_qi(nsQueryInterface, nsID const&amp;) [nsCOMPtr.cpp : 47 + 0x0]
    eip = 0x029d7980   esp = 0xbfffcc10   ebp = 0xbfffcc38   ebx = 0x01ea6be1
    esi = 0xc6f70337   edi = 0xbfffcd54   eax = 0xbfffcd54   ecx = 0x02b7ac04
    edx = 0xc6f70337   efl = 0x00210282
 1  XUL!nsXPConnect::ReparentScopeAwareWrappers(JSContext*, JSObject*, JSObject*) [nsCOMPtr.h : 572 + 0xa]
    eip = 0x01ea6f26   esp = 0xbfffcc40   ebp = 0xbfffcd78
 2  XUL!nsContentUtils::ReparentContentWrappersInScope(nsIScriptGlobalObject*, nsIScriptGlobalObject*) [nsContentUtils.cpp:040cb204cd92 : 1291 + 0x1c]
    eip = 0x02276ddd   esp = 0xbfffcd80   ebp = 0xbfffcdb8
 3  XUL!nsHTMLDocument::OpenCommon(nsACString_internal const&amp;, int) [nsHTMLDocument.cpp:040cb204cd92 : 1918 + 0xb]
    eip = 0x023afc2a   esp = 0xbfffcdc0   ebp = 0xbfffcef8
 4  XUL!nsHTMLDocument::Open(nsACString_internal const&amp;, int, nsIDOMDocument**) [nsHTMLDocument.cpp:040cb204cd92 : 2009 + 0x12]
    eip = 0x023affa1   esp = 0xbfffcf00   ebp = 0xbfffcf28
 5  XUL!nsHTMLDocumentSH::DocumentOpen(JSContext*, JSObject*, unsigned int, long*, long*) [nsDOMClassInfo.cpp:040cb204cd92 : 8295 + 0x1c]
    eip = 0x02495a6d   esp = 0xbfffcf30   ebp = 0xbfffd1c8

then JS code running off a timeout and calling document.open(); entirely expected given the test.

Link to brief log for this test run: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1244163870.1244166645.8960.gz

Comment 1

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix?

So, this might fix this -- if we do a GC while reparenting wrappers, one of them could get collected out from under us.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Don't you need to change MoveableWrapperFinder too?

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

I don't think so... It uses AppendElement, which will create a (temporary) nsRefPtr, which will then be copied into the array and do the Right Thing.

Comment 4

[Boris Zbarsky [:bzbarsky]]

But it'll call the wrong AppendElement, won't it?  That is, it'll call nsTArray<XPCWrappedNative*>::AppendElement.

Also, I just tried running the test in question in a debug m-c build on Mac with gczeal=2.  No crash...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Though I did also have jit disabled; maybe that matters.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Argh

When I wrote the patch, I sort of hoped the compiler would catch errors like that. Thanks, void *s!

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Since bz doesn't get enough bugspam...

...I didn't qrefresh before attaching that last patch.

Comment 8

[Boris Zbarsky [:bzbarsky]]

It worries me that I didn't hit this bug with gczeal==2...  Or at least it makes me worry about the patch fixing it.

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Yeah, this is a total guess on my part -- I'm not entirely sure that a GC can happen here. Do note that in order to crash here, GC would have to happen at the exact right time and a transient reference (through either JS or C++) must not keep the wrapped native alive (so, e.g. whether there's a random event on the event queue or not might matter).

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 382370 [details] [diff] [review]
Since bz doesn't get enough bugspam...

Yeah, ok.  Let's do this and see how it goes.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/6c2c56e0f7ee