Closed Bug 480851 Opened 11 years ago Closed 2 years ago

ASSERTION: "You can't dereference a NULL nsCOMPtr with operator->()" when xpconnect checks thread bits after threads are gone [@ DEBUG_CheckWrapperThreadSafety]

Categories

(Core :: XPConnect, defect, critical)

defect
Not set
critical

Tracking

()

RESOLVED INACTIVE
Tracking Status
blocking2.0 --- -

People

(Reporter: whimboo, Assigned: timeless)

References

()

Details

(Keywords: assertion, crash, hang)

Crash Data

Attachments

(3 files, 2 obsolete files)

Attached file gdb stack on shutdown
Running the test from bug 479490 comment 0 hangs (crashes) the browser on exit. Using a debug build of Minefield I get the above assertion. It cannot reproduce it on 1.9.1 so far.

Steps:
1. Open http://mozilla.pettay.fi/xhr_upload/xhr_upload_demo_sync.html
2. Click on some button at the bottom to load those pages
3. Click on 'new XmlHTTPRequest'
4. Wait for the resulting alert message
5. Exit the browser

###!!! ASSERTION: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../../../dist/include/xpcom/nsCOMPtr.h, line 796

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_PROTECTION_FAILURE at address: 0x00000000
0x112267f8 in DEBUG_CheckWrapperThreadSafety (wrapper=0x1da4a010) at /data/mozilla/build/mozilla-central/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:3390
3390	    else if(NS_SUCCEEDED(wrapper->mThread->IsOnCurrentThread(&val)) && !val)

#0  0x112267f8 in DEBUG_CheckWrapperThreadSafety (wrapper=0x1da4a010) at /data/mozilla/build/mozilla-central/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:3390
#1  0x111e5600 in XPCCallContext::XPCCallContext (this=0xbfff99e0, callerLanguage=XPCContext::LANG_JS, cx=0x842e00, obj=0x127b3000, funobj=0x0, name=310071756, argc=4294967295, argv=0x0, rval=0x0) at /data/mozilla/build/mozilla-central/mozilla/js/src/xpconnect/src/xpccallcontext.cpp:154

More frames are given by the attachment.
Whiteboard: [sg:investigate]
Debug only, and note jsd on the stack. You using firebug?
Assignee: general → nobody
Component: JavaScript Engine → XPConnect
QA Contact: general → xpconnect
Summary: ASSERTION: You can't dereference a NULL nsCOMPtr with operator->() [@ XPCCallContext::XPCCallContext] → ASSERTION: You can't dereference a NULL nsCOMPtr with operator->() [@ DEBUG_CheckWrapperThreadSafety]
Yes, seems like Firebug causes this assertion. Running with a profile where Firebug isn't installed I cannot see this assertion. Adding John to the CC list.
Summary: ASSERTION: You can't dereference a NULL nsCOMPtr with operator->() [@ DEBUG_CheckWrapperThreadSafety] → ASSERTION: "You can't dereference a NULL nsCOMPtr with operator->()" with Firebug installed [@ DEBUG_CheckWrapperThreadSafety]
I'm changing that testcase all the time (may have changed it already).
Henrik, can you reproduce the crash with the current version?
If yes, then I'll copy it to somewhere and start modifying only the
copy.
The only way mThread could be null is if do_GetCurrentThread fails, and it looks like that could only realistically happen if the wrapper is created after XPCOM has locked down the service manager at shutdown. What is firebug doing in its 'script destroyed' hook?
(In reply to comment #3)
> If yes, then I'll copy it to somewhere and start modifying only the
> copy.

Yeah, I can reproduce it with the latest version. Please attach it here as testcase or place it on a safe place. Thanks.
(In reply to comment #4)
> What is firebug doing in
> its 'script destroyed' hook?

Which version of firebug?

http://code.google.com/p/fbug/source/browse/branches/firebug1.4/components/firebug-service.js#1557

Maybe the hook is not being removed before shutdown.

Install the tracing version, 1.4X, open Firebug icon menu tracing window, set
Options FBS_ERRORS, and pref extensions.firebug-tracing-service.toOSConsole
true.

You should see the shutdown messages on window.dump(). Maybe it will give some
hints.
Oh, my fault. Somehow this profile had installed Firebug 1.2.1. I've upgraded to Firebug 1.4Xa12 now.

When opening the given web page I get a lot of assertions and the program quits with a sigtrap.

###!!! ASSERTION: Inner window detected in Equality hook!: 'win->IsOuterWindow()', file /data/mozilla/build/mozilla-central/mozilla/dom/src/base/nsDOMClassInfo.cpp, line 6673
###!!! ASSERTION: Inner window detected in Equality hook!: 'other->IsOuterWindow()', file /data/mozilla/build/mozilla-central/mozilla/dom/src/base/nsDOMClassInfo.cpp, line 6679

#0  JS_Assert (s=0x1bcc5c78 "0", file=0x1bcc5f5c "/data/mozilla/build/mozilla-central/mozilla/js/jsd/jsd_scpt.c", ln=806) at /data/mozilla/build/mozilla-central/mozilla/js/src/jsutil.cpp:62
#1  0x1bcb4564 in jsd_ClearExecutionHook (jsdc=0x1a8cf3d0, jsdscript=0x1dd25090, pc=500310864) at /data/mozilla/build/mozilla-central/mozilla/js/jsd/jsd_scpt.c:806
#2  0x1bcaf016 in JSD_ClearExecutionHook (jsdc=0x1a8cf3d0, jsdscript=0x1dd25090, pc=500310864) at /data/mozilla/build/mozilla-central/mozilla/js/jsd/jsdebug.c:533
#3  0x1bcba656 in jsdScript::ClearBreakpoint (this=0x1dd25890, aPC=0) at /data/mozilla/build/mozilla-central/mozilla/js/jsd/jsd_xpc.cpp:1433
#4  0x004be4eb in NS_InvokeByIndex_P (that=0x1dd25890, methodIndex=30, paramCount=1, params=0xbfffb464) at /data/mozilla/build/mozilla-central/mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179
Attachment #364801 - Attachment description: gdb stack → gdb stack on shutdown
bent: it hardly matters, xpconnect still has to work at shutdown...
Blocks: 364864
I ran into the actual crash here (xpccallcontext.cpp:155) with the fix for bug 364864 - in debug builds, it dies on shutdown while trying to QI a JS component...
I can provide Linux gdb stacks, if someone feels they're helpful.
No longer blocks: 364864
Attached patch comment out crashing check (obsolete) — Splinter Review
Can't we just remove the offending line in xpccallcontext.cpp:

197        DEBUG_CheckWrapperThreadSafety(mWrapper);

It's not used in releases, only in debug builds and test boxes.
It doesn't lead to an error code you could deal with but crashes the whole application. 
That's not particularily useful, given that patches get rejected due to failing tests...
Attachment #405669 - Flags: superreview?(bzbarsky)
Attachment #405669 - Flags: review?
Attachment #405669 - Flags: review? → review?(benjamin)
Comment on attachment 405669 [details] [diff] [review]
comment out crashing check

> It doesn't lead to an error code you could deal
> with but crashes the whole application. 

Yes, this is normal and expected behavior for assertion failures.

> That's not particularily useful,
> given that patches get rejected due to failing
tests...

If patches are being rejected due to failing assertions in a debug build (which is what we're talking about here), then they _should_ have been rejected, unless you can prove the assertion is asserting something that just doesn't have to be true.

In this case, it sounds like at the very least firebug is running various script after XPCOM shutdown and probably shouldn't be doing that, no?  Has someone tried fixing the "unregister the hooks on shutdown" bug in jsd and seeing how that affects things here?
Attachment #405669 - Flags: superreview?(bzbarsky) → superreview-
(In reply to comment #11)
...
> In this case, it sounds like at the very least firebug is running various
> script after XPCOM shutdown and probably shouldn't be doing that, no?  Has
> someone tried fixing the "unregister the hooks on shutdown" bug in jsd and
> seeing how that affects things here?

We don't normally crash on exit in Firebug, so I'm not sure what scripts you think we are running. We try to unhook on quit application granted:
http://code.google.com/p/fbug/source/browse/branches/firebug1.5/components/firebug-service.js#2851
As you can see there we have tracing for this so the procedure from comment #6 will determine if the shutdown code is run before the crash.
Attachment #405669 - Flags: review?(benjamin)
Attachment #405669 - Flags: review-
I have a patch for this which is slowly winding through try server. Please note that this crash has been reported before in bug 364864. Hopefully patch will be up in an hour or so.
Assignee: nobody → timeless
Summary: ASSERTION: "You can't dereference a NULL nsCOMPtr with operator->()" with Firebug installed [@ DEBUG_CheckWrapperThreadSafety] → ASSERTION: "You can't dereference a NULL nsCOMPtr with operator->()" when xpconnect checks thread bits after threads are gone [@ DEBUG_CheckWrapperThreadSafety]
this teaches nsXPConnect about when xpcom has shut down threading. From here, xpconnect can stop trying to enforce rules it can't understand.
Attachment #405669 - Attachment is obsolete: true
Attachment #411738 - Flags: review?
Attachment #411738 - Flags: review? → review?(benjamin)
Status: NEW → ASSIGNED
Comment on attachment 411738 [details] [diff] [review]
stop playing cop when xpcom stops befriending

MozillaTry objects. I need to figure out what went wrong.
Attachment #411738 - Flags: review?(benjamin) → review-
(In reply to comment #15)
> (From update of attachment 411738 [details] [diff] [review])
> MozillaTry objects. I need to figure out what went wrong.

Did you ever figure this out? I don't suppose the logs are still around, as it's been 3 weeks? I'd like to get some traction on this, for the sake of bug 364864 :-(.
Whiteboard: [sg:investigate]
blocking2.0: --- → ?
Not blocking 1.9.3 on this unless this starts to show up in large numbers in the wild.
blocking2.0: ? → -
This is a working version of timeless' attachment #411738 [details] [diff] [review], against trunk. 

I also threw this + patch 364864 + test 364864 onto the tryserver twice (http://hg.mozilla.org/try/pushloghtml?changeset=1ba09ac00827 and http://hg.mozilla.org/try/pushloghtml?changeset=8fb27d137da1) which doesn't seem to be unhappy (apart from random unrelated problems, if I don't misread the tinderboxpushlog), hence requesting review.
Attachment #411738 - Attachment is obsolete: true
Attachment #449554 - Flags: superreview?(bzbarsky)
Attachment #449554 - Flags: review?(bzbarsky)
OS: Mac OS X → All
Hardware: x86 → All
Comment on attachment 449554 [details] [diff] [review]
watch xpcom-shutdown-threads, as proposed

1)  Since the observer service holds a strong ref to nsXPConnect with this patch, removing in the dtor makes no sense.  Removing in the thread shutdown notification would be more sensible.

2)  The static member should be sThreadsGone.

3)  We should set sThreadsGone to PR_FALSE at the point when we add our observer.
Attachment #449554 - Flags: superreview?(bzbarsky)
Attachment #449554 - Flags: superreview-
Attachment #449554 - Flags: review?(bzbarsky)
Attachment #449554 - Flags: review-
Crash Signature: [@ DEBUG_CheckWrapperThreadSafety]
Per policy at https://wiki.mozilla.org/Bug_Triage/Projects/Bug_Handling/Bug_Husbandry#Inactive_Bugs. If this bug is not an enhancement request or a bug not present in a supported release of Firefox, then it may be reopened.
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → INACTIVE
You need to log in before you can comment on or make changes to this bug.