Closed Bug 1300552 Opened 3 years ago Closed 3 years ago
Assertion failure: m
State Data .m Response .is Object() && JS _Is Array Buffer Object(&m State Data .m Response .to Object()), at c:/builds/moz2 _slave/m-cen-w64-d-000000000000000000/build/src/dom/xhr/XMLHttp Request Worker .cpp:2425
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 ?
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
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 firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/2b9f73269c8f XMLHttpRequestWorker::EventRunnable must update stateData also after an abort(), r=ehsan
Hi Baku, should we uplift this to Aurora51, Beta50?
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
Comment on attachment 8796156 [details] [diff] [review] xhr_chrash.patch Fixes a crash in dbg builds, Aurora51+, Beta50+
Duplicate of this bug: 1304702
You need to log in before you can comment on or make changes to this bug.