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

RESOLVED FIXED

Status

()

defect
--
critical
RESOLVED FIXED
10 years ago
a month ago

People

(Reporter: bzbarsky, Assigned: mrbkap)

Tracking

({crash, intermittent-failure})

Trunk
x86
macOS
Points:
---
Bug Flags:
wanted1.9.1 ?

Firefox Tracking Flags

(status1.9.2 beta1-fixed)

Details

(crash signature)

Attachments

(1 attachment, 2 obsolete attachments)

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
Flags: wanted1.9.1?
Flags: blocking1.9.2?
(Reporter)

Updated

10 years ago
Keywords: crash
Whiteboard: [orange]
Posted patch Fix? (obsolete) — Splinter Review
So, this might fix this -- if we do a GC while reparenting wrappers, one of them could get collected out from under us.
Assignee: nobody → mrbkap
Status: NEW → ASSIGNED
Attachment #382199 - Flags: superreview?(bzbarsky)
Attachment #382199 - Flags: review?(bzbarsky)
Don't you need to change MoveableWrapperFinder too?
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.
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...
Though I did also have jit disabled; maybe that matters.
Posted patch Argh (obsolete) — Splinter Review
When I wrote the patch, I sort of hoped the compiler would catch errors like that. Thanks, void *s!
Attachment #382199 - Attachment is obsolete: true
Attachment #382368 - Flags: superreview?(bzbarsky)
Attachment #382368 - Flags: review?(bzbarsky)
Attachment #382199 - Flags: superreview?(bzbarsky)
Attachment #382199 - Flags: review?(bzbarsky)
...I didn't qrefresh before attaching that last patch.
Attachment #382368 - Attachment is obsolete: true
Attachment #382370 - Flags: superreview?(bzbarsky)
Attachment #382370 - Flags: review?(bzbarsky)
Attachment #382368 - Flags: superreview?(bzbarsky)
Attachment #382368 - Flags: review?(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.
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).
(Reporter)

Updated

10 years ago
Attachment #382370 - Flags: superreview?(bzbarsky)
Attachment #382370 - Flags: superreview+
Attachment #382370 - Flags: review?(bzbarsky)
Attachment #382370 - Flags: review+
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.
http://hg.mozilla.org/mozilla-central/rev/6c2c56e0f7ee
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Crash Signature: [@ nsXPConnect::ReparentScopeAwareWrappers]
Whiteboard: [orange]
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.