Last Comment Bug 738535 - FileReader's onload event may cause 'compartment mismatched' assertion
: FileReader's onload event may cause 'compartment mismatched' assertion
Status: RESOLVED FIXED
[mentor=khuey][lang=c++][good first bug]
:
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla14
Assigned To: Mark Hammond [:markh]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-22 20:31 PDT by Mark Hammond [:markh]
Modified: 2012-03-24 13:41 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
test case to demonstrate assertion. (1.83 KB, application/x-javascript)
2012-03-22 20:31 PDT, Mark Hammond [:markh]
no flags Details
JS_WrapValue the array buffer result (729 bytes, patch)
2012-03-22 22:30 PDT, Mark Hammond [:markh]
khuey: review+
Details | Diff | Splinter Review

Description Mark Hammond [:markh] 2012-03-22 20:31:36 PDT
Created attachment 608588 [details]
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-03-22 22:01:27 PDT
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] 2012-03-22 22:30:06 PDT
Created attachment 608599 [details] [diff] [review]
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 [:khuey] (Exited; not receiving bugmail, email if necessary) 2012-03-22 22:32:51 PDT
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 2012-03-23 03:46:30 PDT
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/mhammond@skippinet.com.au-01392d304470
Comment 5 Mark Hammond [:markh] 2012-03-23 17:09:40 PDT
pushed to inbound - http://hg.mozilla.org/integration/mozilla-inbound/rev/dc11394d4693
Comment 6 Olli Pettay [:smaug] (way behind * queues, especially ni? queue) 2012-03-24 05:25:23 PDT
Should this go to aurora or beta?
Comment 7 Ed Morley [:emorley] 2012-03-24 13:41:12 PDT
https://hg.mozilla.org/mozilla-central/rev/dc11394d4693

Note You need to log in before you can comment on or make changes to this bug.