Closed Bug 937612 Opened 9 years ago Closed 9 years ago

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


(Core :: Networking: HTTP, defect)

Not set



Tracking Status
firefox27 --- verified
firefox28 --- verified


(Reporter: mayhemer, Assigned: mcmanus)


Crash Data


(1 file)

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?
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?
ehasan - do you think profile-change-net-teardown and/or xpcom-shutdown have been observed?
xpcom-shutdown is dispatched here: <> and NS_ProcessPendingEvents() is called right after that (line 662), so yes to both (
Attached patch patch 0Splinter Review
Attachment #831241 - Flags: review?(honzab.moz)
Assignee: nobody → mcmanus
Attachment #831241 - Attachment description: speculative connect after shutdown try: -b do -p linux,linux64,macosx64,win32 -u all[Windows 7] -t none → patch 0
Comment on attachment 831241 [details] [diff] [review]
patch 0

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


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+
Crash Signature: [@ nsContentUtils::IsSystemPrincipal]
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
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+
Honza, are you able to reproduce this anymore?
Flags: needinfo?(honzab.moz)
Flags: needinfo?(honzab.moz)
(In reply to Honza Bambas (:mayhemer) from comment #13)
> No.

You need to log in before you can comment on or make changes to this bug.