Null crash [@ nsContentUtils::IsSystemPrincipal] on null sSecurityManager

VERIFIED FIXED in Firefox 27

Status

()

--
critical
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: mayhemer, Assigned: mcmanus)

Tracking

Trunk
mozilla28
Points:
---

Firefox Tracking Flags

(firefox27 verified, firefox28 verified)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
Happened to me during run of network xpcshell tests (the crashed one was test_seer.js).

Today gum (cache2 on) with some local patches (I believe unrelated), based on m-c@361a24da7112.

sSecurityManager = 0x00000000

>	xul.dll!nsContentUtils::IsSystemPrincipal(nsIPrincipal * aPrincipal=0x02c66af0)  Line 4438 + 0xe bytes	C++
 	xul.dll!nsPermissionManager::TestExactPermissionFromPrincipal(nsIPrincipal * aPrincipal=0x02c66af0, const char * aType=0x13d552c0, unsigned int * aPermission=0x0033f664)  Line 949 + 0x9 bytes	C++
 	xul.dll!nsSiteSecurityService::IsSecureURI(unsigned int aType=0, nsIURI * aURI=0x03a3c7d0, unsigned int aFlags=0, bool * aResult=0x0033f7a3)  Line 477 + 0x37 bytes	C++
 	xul.dll!nsHttpHandler::SpeculativeConnect(nsIURI * aURI=0x03a3c7d0, nsIInterfaceRequestor * aCallbacks=0x03ac044c)  Line 1800 + 0x1c bytes	C++
 	xul.dll!IOServiceProxyCallback::OnProxyAvailable(nsICancelable * request=0x03b2d2c0, nsIURI * aURI=0x03a3c7d0, nsIProxyInfo * pi=0x00000000, tag_nsresult status=NS_OK)  Line 1225	C++
 	xul.dll!nsAsyncResolveRequest::DoCallback()  Line 222	C++
 	xul.dll!nsAsyncResolveRequest::OnQueryComplete(tag_nsresult status=NS_OK, const nsCString & pacString={...}, const nsCString & newPACURL={...})  Line 193	C++
 	xul.dll!ExecuteCallback::Run()  Line 85	C++
 	xul.dll!nsThread::ProcessNextEvent(bool mayWait=false, bool * result=0x0033fa43)  Line 610 + 0x19 bytes	C++
 	xul.dll!NS_ProcessPendingEvents(nsIThread * thread=0x01f290e0, unsigned int timeout=4294967295)  Line 201 + 0x14 bytes	C++
 	xul.dll!mozilla::ShutdownXPCOM(nsIServiceManager * servMgr=0x00000000)  Line 663	C++
 	xul.dll!NS_ShutdownXPCOM(nsIServiceManager * servMgr=0x00000000)  Line 618 + 0x9 bytes	C++
 	xul.dll!XRE_XPCShellMain(int argc=23, char * * argv=0x01f120f4, char * * envp=0x01ec31b8)  Line 1587 + 0x7 bytes	C++
 	xpcshell.exe!NS_internal_main(int argc=30, char * * argv=0x01f120d8, char * * envp=0x01ec31b8)  Line 45 + 0x12 bytes	C++
 	xpcshell.exe!wmain(int argc=30, wchar_t * * argv=0x01f11778)  Line 109 + 0x16 bytes	C++
 	xpcshell.exe!__tmainCRTStartup()  Line 552 + 0x19 bytes	C
 	xpcshell.exe!wmainCRTStartup()  Line 371	C


Or is this nsContentUtils bug with missing sInitialized check?

Comment 1

5 years ago
nsContentUtils::Shutdown() clears out sSecurityManager, and this code runs after nsContentUtils::Shutdown() as far as I can tell, so the crash is quite expected.  ;-)

Shouldn't we avoid calling SpeculativeConnect during shutdown?
(Assignee)

Comment 2

5 years ago
ehasan - do you think profile-change-net-teardown and/or xpcom-shutdown have been observed?

Comment 3

5 years ago
xpcom-shutdown is dispatched here: <http://dxr.mozilla.org/mozilla-central/source/xpcom/build/nsXPComInit.cpp#656> and NS_ProcessPendingEvents() is called right after that (line 662), so yes to both (https://wiki.mozilla.org/XPCOM_Shutdown).
(Assignee)

Comment 5

5 years ago
Created attachment 831241 [details] [diff] [review]
patch 0
Attachment #831241 - Flags: review?(honzab.moz)
(Assignee)

Updated

5 years ago
Assignee: nobody → mcmanus
Status: NEW → ASSIGNED
(Assignee)

Updated

5 years ago
Attachment #831241 - Attachment description: speculative connect after shutdown try: -b do -p linux,linux64,macosx64,win32 -u all[Windows 7] -t none → patch 0
(Reporter)

Comment 6

5 years ago
Comment on attachment 831241 [details] [diff] [review]
patch 0

Review of attachment 831241 [details] [diff] [review]:
-----------------------------------------------------------------

r=honzab.

I have confirmed that nsContentUtils::Shutdown cannot be called sooner then from NS_XPCOM_SHUTDOWN_OBSERVER_ID where nsLayoutStatics::Release() (->? nsLayoutStatics::Shutdown() -> nsContentUtils::Shutdown()) is called (at nsLayoutModule.cpp).

However, in a rare case when some observer added between LayoutShutdownObserver and nsHttpHandler (and this is really often used observer topic!) loops the event queue, we can still potentially crash...

So, not a 100% correct fix.
Attachment #831241 - Flags: review?(honzab.moz) → review+
(Reporter)

Updated

5 years ago
Crash Signature: [@ nsContentUtils::IsSystemPrincipal]

Comment 8

5 years ago
https://hg.mozilla.org/mozilla-central/rev/044eb80709a5
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
(Assignee)

Comment 9

5 years ago
Comment on attachment 831241 [details] [diff] [review]
patch 0

[Approval Request Comment]
Bug caused by (feature/regressing bug #):  long standing bug, but 881804 (ff27) will make it more prominent
User impact if declined: crash risk during shutdown
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): extremely low
String or IDL/UUID changes made by this patch: none

Its a crash bug, so its good to backport a safe fix at least to the one release that is likely to trigger it (ff27)
Attachment #831241 - Flags: approval-mozilla-aurora?
Comment on attachment 831241 [details] [diff] [review]
patch 0

low risk, null check helping fix a crash. Looks good to land
Attachment #831241 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 11

5 years ago
  https://hg.mozilla.org/releases/mozilla-aurora/rev/9cf07c05aace
status-firefox27: --- → fixed
status-firefox28: --- → fixed
Honza, are you able to reproduce this anymore?
Flags: needinfo?(honzab.moz)
(Reporter)

Comment 13

5 years ago
No.
Flags: needinfo?(honzab.moz)
(In reply to Honza Bambas (:mayhemer) from comment #13)
> No.

Thanks!
Status: RESOLVED → VERIFIED
status-firefox27: fixed → verified
status-firefox28: fixed → verified
You need to log in before you can comment on or make changes to this bug.