Closed Bug 1300552 Opened 8 years ago Closed 8 years ago

Assertion failure: mStateData.mResponse.isObject() && JS_IsArrayBufferObject(&mStateData.mResponse.toObject()), at c:/builds/moz2_slave/m-cen-w64-d-000000000000000000/build/src/dom/xhr/XMLHttpRequestWorker.cpp:2425

Categories

(Core :: DOM: Workers, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox49 + wontfix
firefox50 + fixed
firefox51 + fixed
firefox52 --- fixed

People

(Reporter: cbook, Assigned: baku)

References

()

Details

(Keywords: assertion)

Attachments

(2 files)

Attached file stack
Assertion failure: mStateData.mResponse.isObject() && JS_IsArrayBufferObject(&mStateData.mResponse.toObject()), at c:/builds/moz2_slave/m-cen-w64-d-000000000000000000/build/src/dom/xhr/XMLHttpRequestWorker.cpp:2425

found by bughunter and reproduced on latest windows trunk debug tinderbox build. 

Seems this is also hitting beta opt builds.

Steps to reproduce:
-> Load 
http://www.doeal.com.br/portal/visualizacoes/pdf/#/p:33/e:27190
--> assertion
[Tracking Requested - why for this release]:

no idea how frequent this crash is, but according to bughunter this affects also beta opt builds
baku, do you know how bad this is ?
Flags: needinfo?(amarchesini)
do you have a crash id or crash signature?
Track for 49+/50+/51+ as it affects all branches.
I just hit this assertion twice on google maps, after searching for "Alamere Falls, California" and then zooming out.  But after that I couldn't reproduce it again.

> Assertion failure: mStateData.mResponse.isObject() && JS_IsArrayBufferObject(&mStateData.mResponse.toObject()), at /home/dbaron/builds/ssd/mozilla-central/mozilla/dom/xhr/XMLHttpRequestWorker.cpp:2435 in UpdateState
> #01: mozilla::dom::(anonymous namespace)::EventRunnable::WorkerRun(JSContext*, mozilla::dom::workers::WorkerPrivate*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/dom/xhr/XMLHttpRequestWorker.cpp:1313)
> #02: mozilla::dom::workers::WorkerRunnable::Run() (/home/dbaron/builds/ssd/mozilla-central/mozilla/dom/workers/WorkerRunnable.cpp:375)
> #03: nsThread::ProcessNextEvent(bool, bool*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/xpcom/threads/nsThread.cpp:1059)
> #04: NS_ProcessNextEvent(nsIThread*, bool) (/home/dbaron/builds/ssd/mozilla-central/mozilla/xpcom/glue/nsThreadUtils.cpp:290)
> #05: mozilla::dom::workers::WorkerPrivate::RunCurrentSyncLoop() (/home/dbaron/builds/ssd/mozilla-central/mozilla/dom/workers/WorkerPrivate.cpp:5403)
> #06: mozilla::dom::workers::WorkerMainThreadRunnable::Dispatch(mozilla::ErrorResult&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/dom/workers/WorkerRunnable.cpp:586)
> #07: mozilla::dom::WorkerThreadProxySyncRunnable::Dispatch(mozilla::ErrorResult&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/dom/xhr/XMLHttpRequestWorker.cpp:214)
> #08: Release (/home/dbaron/builds/ssd/mozilla-central/obj/firefox-debugopt/dist/include/mozilla/RefPtr.h:40)
> #09: mozilla::dom::XMLHttpRequestBinding::abort(JSContext*, JS::Handle<JSObject*>, mozilla::dom::XMLHttpRequest*, JSJitMethodCallArgs const&) (/home/dbaron/builds/ssd/mozilla-central/obj/firefox-debugopt/dom/bindings/XMLHttpRequestBinding.cpp:820)
> #10: mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (/home/dbaron/builds/ssd/mozilla-central/mozilla/dom/bindings/BindingUtils.cpp:2814)
> #11: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/jscntxtinlines.h:236)
> #12: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (/home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/vm/Interpreter.cpp:454)
> #13: InternalCall(JSContext*, js::AnyInvokeArgs const&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/vm/Interpreter.cpp:500)
> #14: Interpret(JSContext*, js::RunState&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/vm/Interpreter.cpp:2910)
> #15: js::RunScript(JSContext*, js::RunState&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/vm/Interpreter.cpp:371)
> #16: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (/home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/vm/Interpreter.cpp:472)
> #17: InternalCall(JSContext*, js::AnyInvokeArgs const&) (/home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/vm/Interpreter.cpp:500)
> #18: js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>) (/home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/jit/BaselineIC.cpp:5998)
> #19: ??? (???:???)
> 
> Program /home/dbaron/builds/fraser/firefox-debugopt/plugin-container (pid = 19858) received signal 11.
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attached patch xhr_chrash.patchSplinter Review
Here what's happen:

1. we have an XHR in worker (yeah), responseType=arrayBuffer
2. XHRWorker receives events from XHRMainThread. One of this is the readyStateChange (state == Done).
3. At this point, in the CTOR of EventRunnable we structuredClone the arrayBuffer and we set the flag mUseCachedArrayBufferResponse in mProxy in order to avoid to clone it for each next message. (Runnable A)
4. In the meantime, in the worker we call XHR.abort()
5. Runnable A is executed in the worker thread. We find that abort() has been called and we don't deserialize the arrayBuffer and we don't set it into mResponse (the bug).
6. XHRMainThread dispatch loadend (or progress event). We create a new EventRunnable (Runnable B), but we don't clone the arrayBuffer because mUseCachedArrayBufferResponse is set to true.
7. We dispatch Runnable B to the worker thread. We try to update the state of the XHR-Worker but mResponse is not correctly set (the crash).

What this patch does is: the state of the worker is always updated by the EventRunnable.
Then, after the update of the state, we decide if we want to dispatch or not the event.
Attachment #8796156 - Flags: review?(ehsan)
Comment on attachment 8796156 [details] [diff] [review]
xhr_chrash.patch

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

Can you please add a test that does the detailed set of steps you commented about above?
Attachment #8796156 - Flags: review?(ehsan) → review+
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2b9f73269c8f
XMLHttpRequestWorker::EventRunnable must update stateData also after an abort(), r=ehsan
https://hg.mozilla.org/mozilla-central/rev/2b9f73269c8f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
Hi Baku, should we uplift this to Aurora51, Beta50?
Flags: needinfo?(amarchesini)
Comment on attachment 8796156 [details] [diff] [review]
xhr_chrash.patch

Approval Request Comment
[Feature/regressing bug #]: XHR in workers
[User impact if declined]: a crash can occur in debug builds
[Describe test coverage new/current, TreeHerder]: green on try
[Risks and why]: this patch is simple enough to land everywhere. It moves a check about having or not hving already seen an event after processing the content of a runnable. This is important in order to propagate the Response Object from main-thread to worker thread.
[String/UUID change made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8796156 - Flags: approval-mozilla-beta?
Attachment #8796156 - Flags: approval-mozilla-aurora?
Comment on attachment 8796156 [details] [diff] [review]
xhr_chrash.patch

Fixes a crash in dbg builds, Aurora51+, Beta50+
Attachment #8796156 - Flags: approval-mozilla-beta?
Attachment #8796156 - Flags: approval-mozilla-beta+
Attachment #8796156 - Flags: approval-mozilla-aurora?
Attachment #8796156 - Flags: approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.