127185 - Running with no .xpt files crashes on launch

Status: RESOLVED FIXED

(Core :: Security: CAPS, defect)

Description

[Simon Fraser [no longer active]]

If you run Mozilla with no .xpt files in the components directory, we crash on
launch, with this stack:

#1   0x02c45944 in js_AtomizeString
 #2   0x02c45be0 in js_Atomize
 #3   0x02c3f6cc in JS_InternString
 #4   0x02dacf64 in nsScriptSecurityManager::_ct(void)
 #5   0x02dad694 in nsScriptSecurityManager::GetScriptSecurityManager(void)
 #6   0x02d98648 in Construct_nsIScriptSecurityManager(nsISupports *, nsID const
&, void **)
 #7   0x00299b18 in nsGenericFactory::CreateInstance(nsISupports *, nsID const
&, void **)
 #8   0x002ab924 in CreateInstanceByContractID__22nsComponentManagerImplFPCcP11nsI
 #9   0x002ae08c in nsComponentManagerImpl::GetServiceByContractID(char const *,
nsID const &, void **)
 #10  0x002a2e3c in nsGetServiceByContractID::_cl( const(nsID const &, void **))
 #11  0x03c0267c in assign_from_helper__36nsCOMPtr<24nsIScriptSecurityManager>FRC1
 #12  0x03bfa650 in mozJSComponentLoader::ReallyInit(void)
 #13  0x03bfdfdc in 0x3bfdfdc
 #14  0x03bfc398 in 0x3bfc398
 #15  0x03bfba60 in mozJSComponentLoader::AutoRegisterComponent(int, nsIFile *,
int *)
 #16  0x03bfad10 in mozJSComponentLoader::RegisterComponentsInDir(int, nsIFile *)
 #17  0x03bfa9e8 in mozJSComponentLoader::AutoRegisterComponents(int, nsIFile *)
 #18  0x002b2e40 in nsComponentManagerImpl::AutoRegisterImpl(int, nsIFile *, int)
 #19  0x002b1c98 in nsComponentManagerImpl::AutoRegister(int, nsIFile *)
 #20  0x002b6944 in nsComponentManager::AutoRegister(int, nsIFile *)
 #21  0x001ad604 in 0x1ad604
 #22  0x001b04a4 in main

The JS context ('cx') is nil.

Comment 1

[John Bandhauer]

This is a crash in caps. nsScriptSecurityManager::GetSafeJSContext can return
null but its callers fail to check for that case. There are certainly plenty of
things that will fail to *work* (including the failure to create component
instances as in this case) when certain typelibs are not present. I wouldn't be
surprised if there are things that will crash like this one. If the goal is to
clean them up then perhaps we ought to fix this case and hand the bug along to
the owner of the next crash location.

Comment 2

[Mitchell Stoltz (not reading bugmail)]

I'm happy to fix security manager, but do we need this to work?

Comment 3

[Mitchell Stoltz (not reading bugmail)]

*** Bug 134988 has been marked as a duplicate of this bug. ***

Comment 4

[timeless]

please fix it :-), i'll happily trigger crashes and stacks for the later broken
callers.

Comment 5

[Simon Fraser [no longer active]]

Attachment: Patch to nsScriptSecurityManager to fix the crash

This patch cleans up nsScriptSecurityManager initalization. It breaks the init
code out of the ctor and into an Init() method (so that a result can be
returned). If no JSContext can be made, then it returns a failure code (which
fixes the crash). I renamed the ssecMan global to gScriptSecMan for clarity,
cleared it in the dtor, and made the ctor and dtor private.

Comment 6

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 109874 [details] [diff] [review]
Patch to nsScriptSecurityManager to fix the crash

>Index: include/nsScriptSecurityManager.h
>+        rv = ssManager->Init();
>+        NS_ASSERTION(NS_SUCCEEDED(rv), "Failed to initialize nsScriptSecurityManager");
>+        return NULL;
>+        
>+        gScriptSecMan = ssManager;

Unless I misread this, it looks like the assignment line will never be reached;
the function will always return null.

Comment 7

[Simon Fraser [no longer active]]

Attachment: Patch fixes an error in GetScriptSecurityManager

Comment 8

[Mitchell Stoltz (not reading bugmail)]

Comment on attachment 109877 [details] [diff] [review]
Patch fixes an error in GetScriptSecurityManager

Looks good. r=mstoltz.

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 109877 [details] [diff] [review]
Patch fixes an error in GetScriptSecurityManager

sr=jst

Comment 10

[Simon Fraser [no longer active]]

Comment on attachment 109874 [details] [diff] [review]
Patch to nsScriptSecurityManager to fix the crash

(obsolete patch)

Comment 11

[Simon Fraser [no longer active]]

Taking for checkin.

Comment 12

[Simon Fraser [no longer active]]

Fix checked in.

Comment 13

[Philip K. Warren]

Is gScriptSecMan ever deleted? I see where it is assigned, and it is set to null
in the destructor, but it is never deleted.

Comment 14

[Simon Fraser [no longer active]]

I don't believe it was ever deleted before, either. I just renamed the static,
and made sure it was nulled out _if_ it was ever deleted.

Comment 15

[Philip K. Warren]

Ok... just wondering.