738535 - FileReader's onload event may cause 'compartment mismatched' assertion

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Mark Hammond [:markh] [:mhammond]]

Attachment: test case to demonstrate assertion.

I'm attaching a test case which can cause attempts to access the '.result' attribute during an onload event in a file reader to assert:

    *** Compartment mismatch 070B0788 vs. 036FD210
    Assertion failure: compartment mismatched, at ...\js\src\jscntxtinlines.h:156

In the test, the following (somewhat unusual) steps are taken:

* Create an iframe.
* Create a sandbox with the same principal as the iframe.
* Inject the FileReader function into the sandbox from the iframe.
* Execute code in the sandbox which uses the FileReader

FWIW, we also use the same technique for WebSocket and this works fine.

The test-case is a bootstrap.js for an extension.  You will also need a trivial install.rdf for this to work - I can attach one if desired.  You should see the assertion/crash at startup in a debug build.  My testing shows the onload event is delivered but the assertion happens when accessing the .result attribute of the FileReader.  The same test from "normal" content (ie, without the sandbox interaction) works correctly.  In a release build the code works correctly (ie, the .result attribute reference returns the correct value)

A snipped stack-trace from MSVC is:

	mozglue.dll!MOZ_Crash()
 	mozglue.dll!MOZ_Assert(const char * s=0x6915785c, const char * file=0x69156834, int ln=156)
 	mozjs.dll!js::CompartmentChecker::fail(JSCompartment * c1=0x0678a868, JSCompartment * c2=0x04c02068)
 	mozjs.dll!js::CompartmentChecker::check(JSCompartment * c=0x04c02068)
 	mozjs.dll!js::CompartmentChecker::check(JSObject * obj=0x05e485e0)
 	mozjs.dll!js::CompartmentChecker::check(const JS::Value & v={...})
 	mozjs.dll!js::assertSameCompartment<JSObject *,JS::Value>...
 	mozjs.dll!js::CallJSPropertyOp(...
 	mozjs.dll!js::ProxyHandler::get(...
 	xul.dll!xpc::XrayWrapper<js::SecurityWrapper<js::CrossCompartmentWrapper> >::get(...
 	mozjs.dll!js::Proxy::get(...
 	mozjs.dll!proxy_GetGeneric(...
 	mozjs.dll!JSObject::getGeneric(...
 	mozjs.dll!JSObject::getGeneric(...
 	mozjs.dll!js::GetPropertyGenericMaybeCallXML(...
 	mozjs.dll!js::GetPropertyOperation(...
 	mozjs.dll!js::Interpret(...
 	mozjs.dll!js::RunScript(...
 	mozjs.dll!js::InvokeKernel(...
 	mozjs.dll!js::Invoke(...
 	mozjs.dll!js::Invoke(...
 	mozjs.dll!JS_CallFunctionValue(...
 	xul.dll!nsXPCWrappedJSClass::CallMethod(...
 	xul.dll!nsXPCWrappedJS::CallMethod(...
 	xul.dll!PrepareAndDispatch(...
 	xul.dll!SharedStub()
 	xul.dll!nsDOMEventListenerWrapper::HandleEvent(nsIDOMEvent * aEvent=0x069a97c0)
 	xul.dll!nsEventListenerManager::HandleEventSubType(...
 	xul.dll!nsEventListenerManager::HandleEventInternal(...
 	xul.dll!nsEventListenerManager::HandleEvent(...
 	xul.dll!nsEventTargetChainItem::HandleEvent(...
 	xul.dll!nsEventTargetChainItem::HandleEventTargetChain(...
 	xul.dll!nsEventDispatcher::Dispatch(...
 	xul.dll!nsEventDispatcher::DispatchDOMEvent(...
 	xul.dll!nsDOMEventTargetHelper::DispatchDOMEvent(...
 	xul.dll!nsDOMFileReader::DispatchDOMEvent(...
 	xul.dll!mozilla::dom::FileIOObject::DispatchProgressEvent(const nsAString_internal & aType={...})
 	xul.dll!mozilla::dom::FileIOObject::OnStopRequest(...
 	xul.dll!nsBaseChannel::OnStopRequest(...
 	xul.dll!nsInputStreamPump::OnStateStop()
 	xul.dll!nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream * stream=0x0567c6f8)
 	xul.dll!nsInputStreamReadyEvent::Run()
 	xul.dll!nsThread::ProcessNextEvent(...
 	xul.dll!NS_ProcessNextEvent_P(...
 	xul.dll!mozilla::ipc::MessagePump::Run(...
 	xul.dll!MessageLoop::RunInternal()
 	xul.dll!MessageLoop::RunHandler()
 	xul.dll!MessageLoop::Run()
 	xul.dll!nsBaseAppShell::Run()
 	xul.dll!nsAppShell::Run()
 	xul.dll!nsAppStartup::Run()
 	xul.dll!XRE_main(...
 	firefox.exe!do_main(...
 	firefox.exe!NS_internal_main(...
 	firefox.exe!wmain(int argc=7, wchar_t * * argv=0x000aec18)
        ...

Comment 1

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

The branch at http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsDOMFileReader.cpp#238 needs to JS_WrapValue the result (much like http://mxr.mozilla.org/mozilla-central/source/content/events/src/nsDOMMessageEvent.cpp#106 does).

Comment 2

[Mark Hammond [:markh] [:mhammond]]

Attachment: JS_WrapValue the array buffer result

Awesome, thanks for the clue.  The attached patch fixes this test case and also the extension from which the test derived.

Comment 3

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

Comment on attachment 608599 [details] [diff] [review]
JS_WrapValue the array buffer result

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

Awesome.

::: content/base/src/nsDOMFileReader.cpp
@@ +242,5 @@
>      } else {
>        *aResult = JSVAL_NULL;
>      }
> +    if (!JS_WrapValue(aCx, aResult))
> +      return NS_ERROR_FAILURE;

Brace your ifs please.

Comment 4

[Mozilla RelEng Bot]

Try run for 01392d304470 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=01392d304470
Results (out of 114 total builds):
    success: 105
    warnings: 9
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-01392d304470

Comment 5

[Mark Hammond [:markh] [:mhammond]]

pushed to inbound - http://hg.mozilla.org/integration/mozilla-inbound/rev/dc11394d4693

Comment 6

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

Should this go to aurora or beta?

Comment 7

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/dc11394d4693