Closed Bug 373219 Opened 18 years ago Closed 18 years ago

###!!! ASSERTION: Fault in cycle collector: null XPCOM pointer returned (ptr: 0)

Categories

(Core :: XPCOM, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: tor, Assigned: peterv)

References

Details

Attachments

(2 files, 2 obsolete files)

Crash in the cycle collector from a tree built from a CVS pull Thu Mar 8 11:20:42 CST 2007.  Bug/stack trace at peterv's request.

###!!! ASSERTION: Fault in cycle collector: null XPCOM pointer returned (ptr: 0)
: 'Not Reached', file /home/tor/moz/trunk/mozilla/xpcom/base/nsCycleCollector.cpp, line 597

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 1076462480 (LWP 31085)]
0x400e2d6a in canonicalize (in=0x0) at /home/tor/moz/trunk/mozilla/xpcom/base/nsCycleCollector.cpp:630
630                            getter_AddRefs(child));
Current language:  auto; currently c++
(gdb) where
#0  0x400e2d6a in canonicalize (in=0x0) at /home/tor/moz/trunk/mozilla/xpcom/base/nsCycleCollector.cpp:630
#1  0x400e2fa4 in GraphWalker::NoteXPCOMChild (this=0xbff78370, child=0x0) at /home/tor/moz/trunk/mozilla/xpcom/base/nsCycleCollector.cpp:641
#2  0x40e36155 in ElementTraverser (key=@0xa672a04, element=0x0, userArg=0xbff78370) at /home/tor/moz/trunk/mozilla/content/html/content/src/nsHTMLFormElement.cpp:569
#3  0x40e3d635 in nsBaseHashtable<nsStringCaseInsensitiveHashKey, nsCOMPtr<nsIDOMHTMLInputElement>, nsIDOMHTMLInputElement*>::s_EnumReadStub (table=0xaafd79c, hdr=0xa672a00, number=0, arg=0xbff7826c)
    at ../../../../dist/include/xpcom/nsBaseHashtable.h:327
#4  0x4004beac in PL_DHashTableEnumerate (table=0xaafd79c, 
    etor=0x40e3d5de <nsBaseHashtable<nsStringCaseInsensitiveHashKey, nsCOMPtr<nsIDOMHTMLInputElement>, nsIDOMHTMLInputElement*>::s_EnumReadStub(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*)>, 
    arg=0xbff7826c) at pldhash.c:683
#5  0x40e3ea32 in nsBaseHashtable<nsStringCaseInsensitiveHashKey, nsCOMPtr<nsIDOMHTMLInputElement>, nsIDOMHTMLInputElement*>::EnumerateRead (this=0xaafd79c, enumFunc=0x40e36130 <ElementTraverser>, 
    userArg=0xbff78370) at ../../../../dist/include/xpcom/nsBaseHashtable.h:188
#6  0x40e3bb03 in nsHTMLFormElement::cycleCollection::Traverse (this=0x41381d2c, s=0xaafd760, cb=@0xbff78370) at /home/tor/moz/trunk/mozilla/content/html/content/src/nsHTMLFormElement.cpp:578
#7  0x400e4271 in nsCycleCollectionXPCOMRuntime::Traverse (this=0x994b9f8, p=0xaafd760, cb=@0xbff78370) at /home/tor/moz/trunk/mozilla/xpcom/base/nsCycleCollector.cpp:984
#8  0x400e2cf6 in GraphWalker::Walk (this=0xbff78370, s0=0xaafd760) at /home/tor/moz/trunk/mozilla/xpcom/base/nsCycleCollector.cpp:700
#9  0x400e366a in nsCycleCollector::MarkRoots (this=0x40132b40) at /home/tor/moz/trunk/mozilla/xpcom/base/nsCycleCollector.cpp:758
#10 0x400e37c5 in nsCycleCollector::Collect (this=0x40132b40) at /home/tor/moz/trunk/mozilla/xpcom/base/nsCycleCollector.cpp:1658
#11 0x400e38c6 in nsCycleCollector_collect () at /home/tor/moz/trunk/mozilla/xpcom/base/nsCycleCollector.cpp:1749
#12 0x40f9b10d in nsJSContext::Notify (this=0xbc04a68, timer=0xbeb6900) at /home/tor/moz/trunk/mozilla/dom/src/base/nsJSEnvironment.cpp:3169
#13 0x400d1a49 in nsTimerImpl::Fire (this=0xbeb6900) at /home/tor/moz/trunk/mozilla/xpcom/threads/nsTimerImpl.cpp:386
#14 0x400d1bcf in nsTimerEvent::Run (this=0x43a18378) at /home/tor/moz/trunk/mozilla/xpcom/threads/nsTimerImpl.cpp:456
#15 0x400cbd82 in nsThread::ProcessNextEvent (this=0x9968638, mayWait=1, result=0xbff78530) at /home/tor/moz/trunk/mozilla/xpcom/threads/nsThread.cpp:482
#16 0x4005a40b in NS_ProcessNextEvent_P (thread=0x9968638, mayWait=1) at nsThreadUtils.cpp:225
#17 0x415a1e8e in nsBaseAppShell::Run (this=0x9a1a918) at /home/tor/moz/trunk/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:153
#18 0x41545c74 in nsAppStartup::Run (this=0x9a18fc0) at /home/tor/moz/trunk/mozilla/xpfe/components/startup/src/nsAppStartup.cpp:218
#19 0x0804dfdd in main1 (argc=1, argv=0xbff78804, nativeApp=0x9995328) at /home/tor/moz/trunk/mozilla/xpfe/bootstrap/nsAppRunner.cpp:1228
#20 0x0804e444 in main (argc=1, argv=0xbff78804) at /home/tor/moz/trunk/mozilla/xpfe/bootstrap/nsAppRunner.cpp:1722
Assignee: nobody → peterv
Attached patch v1 (obsolete) (deleted) — Splinter Review
Attached patch v1.1 (obsolete) (deleted) — Splinter Review
Attachment #257852 - Attachment is obsolete: true
Attachment #257854 - Flags: superreview?(dbaron)
Attachment #257854 - Flags: review?(dbaron)
Comment on attachment 257854 [details] [diff] [review]
v1.1

Well, r+sr=dbaron.  Although !JSVAL_IS_NULL() && JSVAL_IS_OBJECT() could be !JSVAL_IS_PRIMITIVE(), I think.  (Double-check that, though.)

Though I really think we should probably put the null-checks inside the cycle-collector code -- it would save a good bit of code size.  (We could also change, e.g., the TRAVERSE_COMPTR macro.)
Attachment #257854 - Flags: superreview?(dbaron)
Attachment #257854 - Flags: superreview+
Attachment #257854 - Flags: review?(dbaron)
Attachment #257854 - Flags: review+
Blocks: 368773
Status: NEW → ASSIGNED
Attached patch v2 (deleted) — Splinter Review
Attachment #257854 - Attachment is obsolete: true
Attached patch v2 (diff -w) (deleted) — Splinter Review
Let's do this instead.
Attachment #257885 - Flags: superreview?(dbaron)
Attachment #257885 - Flags: review?(dbaron)
OS: Linux → All
Comment on attachment 257885 [details] [diff] [review]
v2 (diff -w)

>Index: xpcom/base/nsCycleCollectionParticipant.h
> #define NS_IMPL_CYCLE_COLLECTION_TRAVERSE_NSCOMPTR_AMBIGUOUS(_field, _base)    \
>-  {                                                                            \
>-    nsISupports *f = NS_ISUPPORTS_CAST(_base*, tmp->_field);                   \
>-    if (f) { cb.NoteXPCOMChild(f); }                                           \
>-  }
>+    cb.NoteXPCOMChild(NS_ISUPPORTS_CAST(_base*, tmp->_field.get()));

No need for the added .get()

>@@ -1532,7 +1532,7 @@ nsCycleCollector::Suspect(nsISupports *n
>     if (!NS_IsMainThread())
>         Fault("trying to suspect from non-main thread");
> 
>-    if (!nsCycleCollector_isScanSafe(n))
>+    if (!n || !nsCycleCollector_isScanSafe(n))
>         Fault("suspected a non-scansafe pointer", n);    
> 
>     if (nsCycleCollector_shouldSuppress(n))
>@@ -1764,9 +1764,6 @@ nsCycleCollector_isScanSafe(nsISupports 
> {
>     nsresult rv;
> 
>-    if (!s)
>-        return PR_FALSE;
>-
>     nsCOMPtr<nsCycleCollectionParticipant> cp = do_QueryInterface(s, &rv);
>     if (NS_FAILED(rv)) {
>         sCollector.mStats.mFailedQI++;

Not sure what the rationale for moving this is; it's probably fine either way, but the way it was before seems ever so slightly better to me.

r+sr=dbaron
Attachment #257885 - Flags: superreview?(dbaron)
Attachment #257885 - Flags: superreview+
Attachment #257885 - Flags: review?(dbaron)
Attachment #257885 - Flags: review+
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: