1278608 - Handle attempts to instantiate JS components on non-main threads somewhat better

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Boris Zbarsky [:bzbarsky]]

This came up in bug 1278605, where PSM tries to instantiate a JS-implemented service on some socket thread.  We end up crashing with a stack like this:

 0 XUL!mozilla::dom::workers::GetCurrentThreadJSContext() [CycleCollectedJSRuntime.h:b0310cb90fd0 : 317 + 0x0]
 1 XUL!<name omitted> [ScriptSettings.cpp:b0310cb90fd0 : 381 + 0x5]
 2 XUL!mozJSComponentLoader::LoadModule(mozilla::FileLocation&) [mozJSComponentLoader.cpp:b0310cb90fd0 : 370 + 0x8]
 3 XUL!nsFactoryEntry::GetFactory() [nsComponentManager.cpp:b0310cb90fd0 : 861 + 0x3]
 4 XUL!nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) [nsComponentManager.cpp:b0310cb90fd0 : 1200 + 0x8]
 5 XUL!nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) [nsComponentManager.cpp:b0310cb90fd0 : 1559 + 0x11]

Now we got lucky.  This was a non-main thread, so when we tried to set up an AutoJSAPI we went to workers::GetCurrentThreadJSContext() which does GetCurrentThreadWorkerPrivate(), which does:

1291   CycleCollectedJSRuntime* ccrt = CycleCollectedJSRuntime::Get();
1274   JSRuntime* rt = ccrt->Runtime();

etc.  I'm not sure where we end up with null here, but it doesn't matter here too much: at some point we do and crash.

But we could have gotten unlucky: if someone tried to instantiate a JS-implemented component on a _worker_ thread, this would all work and we'd just press on here.

Is there a reason we can't just make mozJSComponentLoader::LoadModule bail out if !NS_IsMainThread()?

Comment 1

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

ccrt will be null on any thread that isn't running the cycle collector, which should be all non-DOM threads.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Right, what I'm not sure is why the crash stack claims we crash under the ccrt->Runtime() call given the null -check.  But that's really pretty incidental to the main point of this bug.  ;)

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Ah, I see.  But yes, largely irrelevant.  I agree we should be smarter here.

Comment 4

[Andrew Overholt [:overholt]]

Feel free to change this from backlog if you disagree.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Bobby, see comment 0?

Comment 6

[Bobby Holley (:bholley)]

(In reply to Boris Zbarsky [:bz] from comment #0)
> Is there a reason we can't just make mozJSComponentLoader::LoadModule bail
> out if !NS_IsMainThread()?

SGTM.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Just bail out of mozJSComponentLoader::LoadModule if it's called on a non-main thread

Comment 8

[Bobby Holley (:bholley)]

Comment on attachment 8765010 [details] [diff] [review]
Just bail out of mozJSComponentLoader::LoadModule if it's called on a non-main thread

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

Thanks.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b7c2618bc03
Just bail out of mozJSComponentLoader::LoadModule if it's called on a non-main thread.  r=bholley

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2b7c2618bc03