1649448 - Assertion failure: aGlobal, at /builds/worker/checkouts/gecko/dom/file/Blob.cpp:74

Status: RESOLVED FIXED

(Core :: DOM: File, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.zip

Testcase found while fuzzing mozilla-central rev adc328596e28 (built with --enable-debug).

Assertion failure: aGlobal, at /builds/worker/checkouts/gecko/dom/file/Blob.cpp:74

rax = 0x00007faa9005c1c9   rdx = 0x0000000000000000
rcx = 0x00005615cd253a58   rbx = 0x00007fa9f8076c50
rsi = 0x00007faaa14c28b0   rdi = 0x00007faaa14c1680
rbp = 0x00007faa7d26db50   rsp = 0x00007faa7d26db30
r8 = 0x00007faaa14c28b0    r9 = 0x00007faa7d271700
r10 = 0x0000000000000002   r11 = 0x0000000000000000
r12 = 0x00007fa9f801c280   r13 = 0x00007faa7d26dc00
r14 = 0x0000000000000000   r15 = 0x0000000000000000
rip = 0x00007faa89e32d3f
OS|Linux|0.0.0 Linux 5.3.0-51-generic #44~18.04.2-Ubuntu SMP Thu Apr 23 14:27:18 UTC 2020 x86_64
CPU|amd64|family 6 model 94 stepping 3|8
GPU|||
Crash|SIGSEGV|0x0|33
33|0|libxul.so|mozilla::dom::Blob::Create(nsIGlobalObject*, mozilla::dom::BlobImpl*)|hg:hg.mozilla.org/mozilla-central:dom/file/Blob.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|74|0x29
33|1|libxul.so|mozilla::dom::XMLHttpRequestWorker::GetResponse(JSContext*, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&)|hg:hg.mozilla.org/mozilla-central:dom/xhr/XMLHttpRequestWorker.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|2140|0x18
33|2|libxul.so|mozilla::dom::XMLHttpRequest_Binding::get_response(JSContext*, JS::Handle<JSObject*>, void*, JSJitGetterCallArgs)|s3:gecko-generated-sources:147d96ae793d7536a31803eba17afecba155f261c8ab2f3d2d03fab870f12bf7fcaf453d14c47c9fc7db8e4c7d68e4d517a6d2c0582948e540072d86576b5745/dom/bindings/XMLHttpRequestBinding.cpp:|1778|0x1b
33|3|libxul.so|bool mozilla::dom::binding_detail::GenericGetter<mozilla::dom::binding_detail::NormalThisPolicy, mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*)|hg:hg.mozilla.org/mozilla-central:dom/bindings/BindingUtils.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|3101|0x1c
33|4|libxul.so|CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|484|0x12
33|5|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|576|0xe
33|6|libxul.so|InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|639|0x10
33|7|libxul.so|js::CallGetter(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|780|0x2b
33|8|libxul.so|bool GetExistingProperty<(js::AllowGC)1>(JSContext*, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<js::Shape*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType)|hg:hg.mozilla.org/mozilla-central:js/src/vm/NativeObject.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|2313|0x55
33|9|libxul.so|bool NativeGetPropertyInline<(js::AllowGC)1>(JSContext*, js::MaybeRooted<js::NativeObject*, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::Value, (js::AllowGC)1>::HandleType, js::MaybeRooted<JS::PropertyKey, (js::AllowGC)1>::HandleType, IsNameLookup, js::MaybeRooted<JS::Value, (js::AllowGC)1>::MutableHandleType)|hg:hg.mozilla.org/mozilla-central:js/src/vm/NativeObject.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|2453|0x19
33|10|libxul.so|js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, js::PropertyName*, JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/vm/ObjectOperations-inl.h:adc328596e28636b03fabe701ec6a4d07054e5af|124|0x15
33|11|libxul.so|js::GetProperty(JSContext*, JS::Handle<JS::Value>, JS::Handle<js::PropertyName*>, JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|4687|0x12
33|12|libxul.so|Interpret(JSContext*, js::RunState&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|2978|0x40e
33|13|libxul.so|js::RunScript(JSContext*, js::RunState&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|456|0xb
33|14|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|611|0x8
33|15|libxul.so|InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|639|0x10
33|16|libxul.so|<name omitted>|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|656|0xb
33|17|libxul.so|js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/vm/SelfHosting.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|1689|0x1a
33|18|libxul.so|AsyncFunctionResume(JSContext*, JS::Handle<js::AsyncFunctionGeneratorObject*>, ResumeKind, JS::Handle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/vm/AsyncFunction.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|128|0x11
33|19|libxul.so|PromiseReactionJob(JSContext*, unsigned int, JS::Value*)|hg:hg.mozilla.org/mozilla-central:js/src/builtin/Promise.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|1852|0x58
33|20|libxul.so|CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|484|0x12
33|21|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|576|0xe
33|22|libxul.so|InternalCall(JSContext*, js::AnyInvokeArgs const&, js::CallReason)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|639|0x10
33|23|libxul.so|<name omitted>|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|656|0xb
33|24|libxul.so|JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/jsapi.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|2846|0x23
33|25|libxul.so|mozilla::dom::PromiseJobCallback::Call(mozilla::dom::BindingCallContext&, JS::Handle<JS::Value>, mozilla::ErrorResult&)|s3:gecko-generated-sources:30de989a0be01a566d978da9934fc47a7a1d7e19d87d32dc4bcdab5e85996b3194b6f3bfead795c2ed5279934ac82cd340f2e1afd77a1304954d050c6fc1f374/dom/bindings/PromiseBinding.cpp:|28|0xf
33|26|libxul.so|mozilla::dom::PromiseJobCallback::Call(mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*)|s3:gecko-generated-sources:09cbe7f9e1409cd4cca288356b597724157d7f93ab5efbaede65be8bf535e6469c7590bf6c7211a89f760ea37ac901f3d1d5fcbeb89c9dfc80643c98c831255f/dist/include/mozilla/dom/PromiseBinding.h:|91|0x1e
33|27|libxul.so|mozilla::PromiseJobRunnable::Run(mozilla::AutoSlowOperation&)|hg:hg.mozilla.org/mozilla-central:xpcom/base/CycleCollectedJSContext.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|209|0x41
33|28|libxul.so|mozilla::CycleCollectedJSContext::PerformMicroTaskCheckPoint(bool)|hg:hg.mozilla.org/mozilla-central:xpcom/base/CycleCollectedJSContext.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|644|0x14
33|29|libxul.so|mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*)|hg:hg.mozilla.org/mozilla-central:dom/events/EventListenerManager.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|1090|0x5
33|30|libxul.so|mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event**, mozilla::dom::EventTarget*, nsEventStatus*, bool)|hg:hg.mozilla.org/mozilla-central:dom/events/EventListenerManager.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|1279|0x15
33|31|libxul.so|mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&)|hg:hg.mozilla.org/mozilla-central:dom/events/EventDispatcher.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|355|0xb
33|32|libxul.so|mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)|hg:hg.mozilla.org/mozilla-central:dom/events/EventDispatcher.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|557|0x19
33|33|libxul.so|mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*)|hg:hg.mozilla.org/mozilla-central:dom/events/EventDispatcher.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|1054|0x5
33|34|libxul.so|mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, mozilla::dom::Event*, nsPresContext*, nsEventStatus*)|hg:hg.mozilla.org/mozilla-central:dom/events/EventDispatcher.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|0|0x8
33|35|libxul.so|mozilla::DOMEventTargetHelper::DispatchEvent(mozilla::dom::Event&, mozilla::dom::CallerType, mozilla::ErrorResult&)|hg:hg.mozilla.org/mozilla-central:dom/events/DOMEventTargetHelper.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|145|0xc
33|36|libxul.so|mozilla::dom::EventTarget::DispatchEvent(mozilla::dom::Event&)|hg:hg.mozilla.org/mozilla-central:dom/events/EventTarget.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|178|0x33
33|37|libxul.so|mozilla::dom::MessageEventRunnable::DispatchDOMEvent(JSContext*, mozilla::dom::WorkerPrivate*, mozilla::DOMEventTargetHelper*, bool)|hg:hg.mozilla.org/mozilla-central:dom/workers/MessageEventRunnable.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|106|0x13
33|38|libxul.so|mozilla::dom::WorkerRunnable::Run()|hg:hg.mozilla.org/mozilla-central:dom/workers/WorkerRunnable.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|370|0x16
33|39|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|1234|0xe
33|40|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|504|0xc
33|41|libxul.so|mozilla::dom::WorkerPrivate::DoRunLoop(JSContext*)|hg:hg.mozilla.org/mozilla-central:dom/workers/WorkerPrivate.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|2981|0xd
33|42|libxul.so|mozilla::dom::workerinternals::(anonymous namespace)::WorkerThreadPrimaryRunnable::Run()|hg:hg.mozilla.org/mozilla-central:dom/workers/RuntimeService.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|2216|0xc
33|43|libxul.so|nsThread::ProcessNextEvent(bool, bool*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|1234|0xe
33|44|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|504|0xc
33|45|libxul.so|mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|332|0x13
33|46|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:adc328596e28636b03fabe701ec6a4d07054e5af|316|0x17
33|47|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:adc328596e28636b03fabe701ec6a4d07054e5af|291|0x8
33|48|libxul.so|nsThread::ThreadFunc(void*)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:adc328596e28636b03fabe701ec6a4d07054e5af|447|0x8
33|49|libnspr4.so|_pt_root|hg:hg.mozilla.org/mozilla-central:nsprpub/pr/src/pthreads/ptthread.c:adc328596e28636b03fabe701ec6a4d07054e5af|201|0x7
33|50|libpthread.so.0||||0x76db
33|51|libc.so.6||||0x12188f

Comment 1

[Jens Stutte [:jstutte]]

It seems from this call to Create that we have no owner/parent when we try to create the blob. Can you reproduce this?

Comment 2

[Jason Kratzer [:jkratzer]]

Bugmon Analysis:
Verified bug as reproducible on mozilla-central 20200630144559-933c9f34edfa.
The bug appears to have been introduced in the following build range:
> Start: 90b7c55d811d5eba377002abfef39d2e0fbaf8e5 (20200327224900)
> End: cb814ebff5a56845db97bbed1ef52b0c523661f1 (20200327225135)
> Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=90b7c55d811d5eba377002abfef39d2e0fbaf8e5&tochange=cb814ebff5a56845db97bbed1ef52b0c523661f1

Comment 3

[Jens Stutte [:jstutte]]

The bisect points to bug 1545345. Andrew?

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Is it possible to get a pernosco trace of this?

Marking as a security bug for now because it seems possible the involvement of the microtask checkpoint here may be introducing an edge-case that may impact worker shutdown in a novel way and I'd like to better understand.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

(In reply to Jens Stutte [:jstutte] from comment #1)

It seems from this call to Create that we have no owner/parent when we try to create the blob. Can you reproduce this?

If WorkerPrivate::DestroySyncLoop called ClearMainEventQueue, this would have done DisconnectEventTargetObjects() and resulted in DisconnectFromOwner() which would clear DETH::mParentObject and made GetOwnerGlobal() return null. That really shouldn't have happened yet, but... that would explain the problem.

Comment 6

[Jason Kratzer [:jkratzer]]

Andrew, a Pernosco session is available here: https://pernos.co/debug/KOCH2tsTxugHeYvfrocmcA/index.html

Comment 7

[Andrew Sutherland [:asuth] (he/him)]

Thank you for the pernosco session! Note that unfortunately it looks like pernosco-submit was used without allow-listing the source directory, so there isn't any source code, so if there's automation involved, it should be updated. (I've filed https://github.com/Pernosco/pernosco-submit/issues/8 on making the pernosco-submit script be more helpful by requiring at least one source path unless explicitly directed otherwise via argument. Unfortunately, the lack of source seems to also break unrelated features like using $tid as a "print" specifier, which is also being handled.)

Pernosco shows ClearMainEventQueue was called synchronously as a result of the close() call which is as expected. ClearMainEventQueue now invoking DisconnectEventTargetObjects() at this time was primarily motivated by the desire to avoid calling DisconnectEventTargetObjects with syncloops on the stack. XHR clearly is not prepared for this, which I've previously discussed as a half-close situation where we still sorta need to be able to do things in the global, but we know that nothing inbound can/should ever happen again, so only outbound things should be able to happen.

I need to investigate/think a little more about the spec situation for this and the half-close situation for XHR.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

For sec rating, right now I think the current risk is only nullptr in non-debug build but I continue to investigate.

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

:jkratzer provided a revised pernosco trace at https://pernos.co/debug/ykemVdBo16W7VjwplwTZuA/index.html and indicated bug 1613998 was likely the direct cause of the lack of source. I've added some notebook entries.

For Fx71+ non-debug builds, Blob::Create has a null global guard and returns null but the call to GetOrCreateDOMReflector does not null-check that null and should probably be causing a null de-ref in nsWrapperCache::GetWrapper.

Using the "proto signature" field to look for "GetResponse" as the parent stack frame (search) shows one crash matching this general signature in the past 6 months at https://crash-stats.mozilla.org/report/index/da045dba-2b18-4ef1-bcd7-ff5640200114 which pre-dates any of my changes. (There are, however, a bunch of nsWrapperCache::GetWrapper crashes in the past 6 months, but the the single frame isn't really useful since there's no null-check and null derefs are common. Most seem to be main-thread, with random sampling finding that GetPerformance did happen some on workers.)

Extra investigation shows we're successfully running the sync XHR loop and this is (regrettably) consistent with spec.

On reflection, we definitely want to be deferring DisconnectEventTargetObjects for self.close() until the current task has completed. The question is whether this means deferring ClearMainEventQueue or just the disconnection. The choice of putting the disconnect in CMEQ was that it was semantically consistent and corrected a premature disconnect. The bigger problem is that ClearMainEventQueue doesn't actually do what it thinks it does or the spec says it should do and moving it around could cause real breakage and/or observable changes in behavior, although somewhat edge-casey given the problematic semantics of self.close(). The Cache API continues to be the main concern, so I'm going to try and finish some investigations I previously had underway to clean up this exact scenario.

Comment 10

[Daniel Veditz [:dveditz]]

Has anything changed your opinion since comment 8? Can we unhide this bug or is it still sensitive?

Comment 11

[Andrew Sutherland [:asuth] (he/him)]

This bug should still be hidden for now. The self.close() invariant/ordering issues discussed in the last paragraph of comment 9 are something I'd like to fully finish analyzing and addressing as needed before unhiding this bug out of a surfeit of caution. I should be revisiting this bug over the next several (business) days.

Comment 12

[Andrew McCreight [:mccr8]]

Any updates, Andrew? Thanks.

Comment 13

[Jens Stutte [:jstutte]]

Randell, would you be able to help to give this a spin? Thank you!

Comment 14

[Randell Jesup [:jesup] (needinfo me)]

I'll add it to my queue

Comment 15

[Jens Stutte [:jstutte]]

asuth:, as you are still the assignee here - can we move this forward somehow?

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

Once bug 1800659 removes the calls to ClearMainEventQueue that should potentially clean up the issue, assuming we stop issuing calls to DisconnectEventTargetObjects for self.close(). Marking a dependency on that bug.

Comment 17

[Jens Stutte [:jstutte]]

Now that 1800659 is fixed, let's check.

Comment 18

[Jason Kratzer [:jkratzer]]

Bugmon won't operate on bugs this old unfortunately. I've checked manually and I'm unable to reproduce this with the original testcase. The fuzzers last reported this issue on 2021/01/04. I think we can safely close this.

Comment 19

[Andrew Sutherland [:asuth] (he/him)]

Hooray! Thanks for checking :jkratzer. Indeed in bug 1800659 we really cleaned up our handling of worker termination so we should not see this specific class of failure going forward.