631553 - crash [@ PL_DHashTableOperate | nsFrameScriptExecutor::LoadFrameScriptInternal]

Status: RESOLVED FIXED

(Core :: IPC, defect)

Description

[Josh Matthews [:jdm]]

This bug was filed from the Socorro interface and is 
report bp-049a1da3-0f33-42dc-9150-097b32110203 .
============================================================= 

According to the stack, we're trying to load a frame script in-process due to a DOM event being fired, and meanwhile XPCOM is shutting down.

0 	libxul.so 	PL_DHashTableOperate 	pldhash.c:615
1 	libxul.so 	nsFrameScriptExecutor::LoadFrameScriptInternal 	nsTHashtable.h:171
2 	libxul.so 	nsInProcessTabChildGlobal::LoadFrameScript 	content/base/src/nsInProcessTabChildGlobal.cpp:334
...
37 	libxul.so 	mozilla::ShutdownXPCOM 	xpcom/build/nsXPComInit.cpp:637
38 	libxul.so 	NS_ShutdownXPCOM_P 	xpcom/build/nsXPComInit.cpp:595
39 	libxul.so 	ScopedXPCOMStartup::~ScopedXPCOMStartup 	toolkit/xre/nsAppRunner.cpp:1119

Since this is just the script caching breaking (sCachedScripts is null), I'm hoping we can change a couple lines and avoid this:

>620  nsFrameScriptExecutorJSObjectHolder* holder = sCachedScripts->Get(aURL);

>703              sCachedScripts->Put(aURL, holder);

Comment 1

[Olli Pettay [:smaug][[e-mail]]]

Add a null check?

Comment 2

[Josh Matthews [:jdm]]

Attachment: Avoid caching frame scripts during xpcom shutdown.

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 511039 [details] [diff] [review]
Avoid caching frame scripts during xpcom shutdown.

Could you just return early if !sCachedScripts?

Comment 4

[Josh Matthews [:jdm]]

Do you really want to just not execute the script if we can't cache it?  Granted, this is during shutdown, but it makes me nervous.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

We shouldn't run any new scripts during shutdown, IMO.

Comment 6

[Josh Matthews [:jdm]]

When you say new, do you mean never-seen-before, or just scripts in general?  Because there's no way to determine if a script is new or not, as we've destroyed the script cache.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

I mean we shouldn't allow executing nsFrameScriptExecutor::LoadFrameScriptInternal.
The whole DOM and layout has been already shutdown if sScriptCacheCleaner is null.

Comment 8

[Josh Matthews [:jdm]]

Attachment: Avoid caching frame scripts during xpcom shutdown.

Comment 9

[Josh Matthews [:jdm]]

Attachment: Avoid caching frame scripts during xpcom shutdown.

Comment 10

[Josh Matthews [:jdm]]

Comment on attachment 512823 [details] [diff] [review]
Avoid caching frame scripts during xpcom shutdown.

This is a safe crash-on-shutdown fix that was popping up fairly regularly in b4 crash stats.

Comment 11

[Josh Matthews [:jdm]]

http://hg.mozilla.org/mozilla-central/rev/5534fbe5e318