crash [@ nsScriptSecurityManager::GetCurrentJSContext()]

RESOLVED FIXED in Thunderbird 3.0rc1

Status

MailNews Core
Database
--
critical
RESOLVED FIXED
9 years ago
7 years ago

People

(Reporter: wsmwk, Assigned: Bienvenu)

Tracking

({crash, regression, topcrash})

Trunk
Thunderbird 3.0rc1
x86
All
crash, regression, topcrash
Bug Flags:
blocking-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [no l10n impact], crash signature)

Attachments

(1 attachment)

4.68 KB, patch
neil@parkwaycc.co.uk
: review+
neil@parkwaycc.co.uk
: superreview+
Details | Diff | Splinter Review
(Reporter)

Description

9 years ago
[@ nsScriptSecurityManager::GetCurrentJSContext()]

#18 crash for 3.0b3, but hard to say it's a topcrash given
bp-efccf75b-c216-489f-adec-9b6b02090813 "I've been having trouble all day"
nsScriptSecurityManager::GetCurrentJSContext	 caps/src/nsScriptSecurityManager.cpp:327
nsScriptSecurityManager::IsCapabilityEnabled	caps/src/nsScriptSecurityManager.cpp:2442
nsScriptSecurityManager::CheckXPCPermissions	caps/src/nsScriptSecurityManager.cpp:3082
nsScriptSecurityManager::CanCreateWrapper	caps/src/nsScriptSecurityManager.cpp:2889
XPCWrappedNative::InitTearOff	js/src/xpconnect/src/xpcwrappednative.cpp:1803
XPCWrappedNative::FindTearOff	js/src/xpconnect/src/xpcwrappednative.cpp:1629
XPCCallContext::CanCallNow	js/src/xpconnect/src/xpccallcontext.cpp:270
XPCWrappedNative::CallMethod	js/src/xpconnect/src/xpcwrappednative.cpp:1960
XPC_WN_GetterSetter	js/src/xpconnect/src/xpcwrappednativejsops.cpp:1622
js_Invoke	js/src/jsinterp.cpp:1386
js_InternalInvoke	js/src/jsinterp.cpp:1447
js_InternalGetOrSet	js/src/jsinterp.cpp:1510
js_GetSprop	js/src/jsscope.h:367
js_NativeGet	js/src/jsobj.cpp:4167
js_GetPropertyHelper	js/src/jsobj.cpp:4333
js_Interpret	js/src/jsinterp.cpp:4451
js_Invoke	js/src/jsinterp.cpp:1394
nsXPCWrappedJSClass::CallMethod	js/src/xpconnect/src/xpcwrappedjsclass.cpp:1697
nsXPCWrappedJS::CallMethod	js/src/xpconnect/src/xpcwrappedjs.cpp:561
PrepareAndDispatch	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:114
SharedStub	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:141
nsMsgDBView::OnAnnouncerGoingAway	mailnews/base/src/nsMsgDBView.cpp:5663
nsMsgDatabase::NotifyAnnouncerGoingAway	mailnews/db/msgdb/src/nsMsgDatabase.cpp:729
nsMsgDatabase::ForceClosed	mailnews/db/msgdb/src/nsMsgDatabase.cpp:1224
nsMailDatabase::ForceClosed	mailnews/db/msgdb/src/nsMailDatabase.cpp:118
nsImapMailDatabase::ForceClosed	mailnews/db/msgdb/src/nsImapMailDatabase.cpp:129
nsMsgDatabase::CleanupCache	mailnews/db/msgdb/src/nsMsgDatabase.cpp:763
nsGenericModule::Shutdown	nsGenericFactory.cpp:340
nsGenericModule::`scalar deleting destructor'	
nsGenericModule::Release	nsGenericFactory.cpp:245
nsRefPtr<nsThread>::assign_assuming_AddRef	nsAutoPtr.h:944
nsCOMPtr_base::assign_with_AddRef	nsCOMPtr.cpp:89
info_ClearEntry	xpcom/components/nsStaticComponentLoader.cpp:70
PL_DHashTableFinish	pldhash.c:383
nsComponentManagerImpl::Shutdown	xpcom/components/nsComponentManager.cpp:745
NS_ShutdownXPCOM_P	xpcom/build/nsXPComInit.cpp:856
ScopedXPCOMStartup::~ScopedXPCOMStartup	toolkit/xre/nsAppRunner.cpp:951
XRE_main	toolkit/xre/nsAppRunner.cpp:3342
Assignee: nobody → dveditz
Component: Security → Security: CAPS
Product: Thunderbird → Core
QA Contact: thunderbird → caps

Comment 1

9 years ago
0459:e7774e2e7ca9 (previous:20258) <igor@mir2.org> 2008-10-14 16:16 +0200
Bug 459656 - Implementing nsIThreadJSContextStack in nsXPConnect. r+sr=mrbkap

the stack is very clear:

http://mxr-test.konigsberg.mozilla.org/bonsai/cvsblame.cgi?file=caps/src/nsScriptSecurityManager.cpp&xrev=a787dec4c28a&tree=mozilla1.9.1&mark=3327,3337,327#322

here's the general code path:
3327 nsScriptSecurityManager::Shutdown()
3337 igor@mir2.org 20459 NS_IF_RELEASE(sJSContextStack);

 323 nsScriptSecurityManager::GetCurrentJSContext()
 327 igor@mir2.org 20459 if (NS_FAILED(sJSContextStack->Peek(&cx)))

and here are the key stack frames:
nsScriptSecurityManager::GetCurrentJSConext
NS_ShutdownXPCOM_P

igor: please fix this.
Assignee: dveditz → igor
Blocks: 459656
OS: Windows Vista → All
igor ?
(Reporter)

Comment 4

9 years ago
currently #2, this is shaking out to be a topcrash for 3.0b4
Keywords: topcrash
Why is bug 459656 at fault? If someone has called nsScriptSecurityManager::Shutdown then no one ought to be running script after that. Maybe nsMsgDBView needs to announce on an earlier event, or the script-running listeners should be killed before that point.
STR (copied from 518863) :
Also, if I start TB in normal mode, disable all my extensions, remove Global
Search from the toolbar, restart TB, open a message in tab 2, click on tab 1,
click on Inbox, close TB, TB crashes.
(Reporter)

Comment 7

9 years ago
topcrash for 3.0b4 is now definite. steady at #2, so we would want this fixed for 3.0. labeling tb3needs, but feel free to adjust if appropriate

(In reply to comment #6)
> STR (copied from bug 518863) :
> Also, if I start TB in normal mode, disable all my extensions, remove Global
> Search from the toolbar, restart TB, open a message in tab 2, click on tab 1,
> click on Inbox, close TB, TB crashes.

those comments are from bp-6f29c336-2736-4412-b963-f81402090928
Whiteboard: [tb3needs]
(Assignee)

Comment 8

9 years ago
we're going to need to fix this in our code.
Assignee: igor → bienvenu
Component: Security: CAPS → Database
Product: Core → MailNews Core
QA Contact: caps → database
Target Milestone: --- → Thunderbird 3.0rc1
(Assignee)

Comment 9

9 years ago
I think during the shutdown cleanup caused by our component getting unloaded, we should not be sending these notifications. It's too late...
Flags: blocking-thunderbird3+

Updated

9 years ago
Whiteboard: [tb3needs] → [no l10n impact][tb3needs]
(Assignee)

Comment 10

9 years ago
removing tb3needs
Status: NEW → ASSIGNED
Whiteboard: [no l10n impact][tb3needs] → [no l10n impact]
(Assignee)

Comment 11

9 years ago
It's strange that removing the search widget from the toolbar causes all this grief. It also breaks persistence of tabs, so I suspect there's some code there that tries to access that widget and fails, and throws all the way out, past the code that closes the views for the open tabs, and leaving this dangling view. I'll poke around in a bit.
(Assignee)

Comment 12

9 years ago
now that bug 518863 is fixed, I'll be real curious to see if this stack disappears from nightly builds from tomorrow forward.  I guess any exception that prevented us from closing views on shutdown could cause this crash, so we might need to figure out some way to make the db stop sending notifications once we've gotten past a certain point in shutdown. But I'm not sure that the crash can't occur before we get to the module shutdown phase - I see a ton of assertions if I just close the 3 pane UI with an other mail window open, if I don't have a search bar.

But, CleanupCache is not really useful anymore, so it should probably get turned off. Its main purpose was to shake out memory leaks, but it's no good if it's going to crash.
(Assignee)

Comment 13

9 years ago
Created attachment 403806 [details] [diff] [review]
proposed fix

by the time we get to unloading modules, it's too late to be sending out notifications, because js code might be gone, etc. It was also hiding some memory leaks/bloat...see Bug 519736
Attachment #403806 - Flags: superreview?(neil)
Attachment #403806 - Flags: review?(neil)
(Assignee)

Updated

9 years ago
Whiteboard: [no l10n impact] → [no l10n impact][has patch for review neil]
Comment on attachment 403806 [details] [diff] [review]
proposed fix

>+#ifdef DEBUG
>+    // If you hit this warning, it means that some code is holding onto
>+    // a db at shutdown.
Nit: Doesn't need to be #ifdef because the only caller is already #ifdef
Attachment #403806 - Flags: superreview?(neil)
Attachment #403806 - Flags: superreview+
Attachment #403806 - Flags: review?(neil)
Attachment #403806 - Flags: review+
(Reporter)

Comment 15

9 years ago
(In reply to comment #12)
> now that bug 518863 is fixed, I'll be real curious to see if this stack
> disappears from nightly builds from tomorrow forward.  I guess any exception
> that prevented us from closing views on shutdown could cause this crash, so we
> might need to figure out some way to make the db stop sending notifications
> once we've gotten past a certain point in shutdown

one crash thus far 20091001 build http://crash-stats.mozilla.com/report/index/2c48c79d-ad9f-4729-b3ac-ce0ae2091002
(Assignee)

Comment 16

9 years ago
(In reply to comment #14)

> Nit: Doesn't need to be #ifdef because the only caller is already #ifdef

I addressed that nit by changing that - we should still delete m_dbCache in release mode, because we don't want shutdown memory leaks...fix checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Whiteboard: [no l10n impact][has patch for review neil] → [no l10n impact]
(Assignee)

Updated

9 years ago
Duplicate of this bug: 518671
Crash Signature: [@ nsScriptSecurityManager::GetCurrentJSContext()]
You need to log in before you can comment on or make changes to this bug.