855127 - TestShell environments in content children cannot register nsIMessageListenerManager listeners

Status: RESOLVED WONTFIX

(Core :: IPC, defect)

Description

[Jim Blandy :jimb]

Attachment: Test case. Run with xpcshell; child process crashes

xpcshell has a function for creating content child processes and communicating with them, called 'sendCommand'. This function sends an IPDL PTestShell constructor message to the child, which creates an XPCShellEnvironment in which to evaluate JavaScript sent from the parent. sendCommand then sends a PTestShell::ExecuteCommand message to actually evaluate some code in the child.

The 'sendCommand' function is the only way to exercise child processes in xpcshell tests. However, if code run in the child process using 'sendCommand' registers listeners with nsIMessageListenerManager instances, and those listeners use the global functions defined by XPCShellEnvironment, the child process will crash.

XPCShellEnvironment::Init creates a JSContext and sets its "private" pointer (with JS_SetContextPrivate) to point to the XPCShellEnvironment. It then creates a global object and populates it with some native functions that expect to be able to find the XPCShellEnvironment stored in the "private" pointer of the JSContext they were passed.

If code passed to sendCommand registers a listener with some process message manager (like @mozilla.org/childprocessmessagemanager;1), and a message arrives, then the event loop tries to run the listener. However, process message managers run listeners using a JSContext obtained from nsContentUtils::ThreadJSContextStack()->GetSafeJSContext(), not the JSContext the XPCShellEnvironment was created with. If that listener uses any of the primitives defined by the XPCShellEnvironment, they will fetch the "private" pointer of a different JSContext, and try to interpret it as an XPCShellEnvironment, leading to a crash.

What's kind of silly is that everything that the native functions need XPCShellEnvironment for would (today; I know things were different before) be better obtained elsewhere. The context should come from GetSafeJSContext; the 'quitting' flag and exit code are ignored; the principals could come from the callee's compartment.

The attached code, if run with xpcshell, causes the child process to segfault trying to use a null pointer as an XPCShellEnvironment pointer.

Comment 1

[Jim Blandy :jimb]

Attachment: Make XPCShellEnvironment a singleton, rather than finding it via the JSContext's private pointer.

Patch. I still need to integrate the test (some light hacking required, since 'quit' is also broken).

Comment 2

[Jim Blandy :jimb]

Attachment: Make XPCShellEnvironment a singleton, rather than finding it via the JSContext's private pointer.

Complete patch, including test case. Try:
https://tbpl.mozilla.org/?tree=Try&rev=27836ef67e89

Comment 3

[Jim Blandy :jimb]

Comment on attachment 732622 [details] [diff] [review]
Make XPCShellEnvironment a singleton, rather than finding it via the JSContext's private pointer.

Try push looks okay.

Comment 4

[Jim Blandy :jimb]

Comment on attachment 732622 [details] [diff] [review]
Make XPCShellEnvironment a singleton, rather than finding it via the JSContext's private pointer.

Removing review request, as there's a B2G xpcshell failure I'd failed to notice in the Try push which definitely looks related. I'll sort that out before asking for review again.

Comment 5

[Jim Blandy :jimb]

Added some diagnostics, trying that failing test alone:
https://tbpl.mozilla.org/?tree=Try&rev=c5fe8079226f

Comment 6

[Jim Blandy :jimb]

With the patch in bug 874755, we may not need TestShell and XPCShellEnvironment at all any more, so working on XPCShellEnvironment is probably not worth it.