1543154 - Assertion failure: mGlobal, at /builds/worker/workspace/build/src/dom/indexedDB/IDBFactory.cpp:571

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect, P2)

Description

[Jason Kratzer [:jkratzer]]

Attachment: testcase.html

Testcase found while fuzzing mozilla-central rev 98b223de0543.

rax = 0x0000557eda7b5e20 rdx = 0x0000000000000000
rcx = 0x0000000000000b40 rbx = 0x00007f3248520545
rsi = 0x00007f325361a8b0 rdi = 0x00007f3253619680
rbp = 0x00007ffd7f6e08e0 rsp = 0x00007ffd7f6e05c0
r8 = 0x00007f325361a8b0 r9 = 0x00007f3254777740
r10 = 0x0000000000000002 r11 = 0x0000000000000000
r12 = 0x00007ffd7f6e0918 r13 = 0x00007ffd7f6e0af0
r14 = 0x00007ffd7f6e09d8 r15 = 0x00007ffd7f6e09c0
rip = 0x00007f3244ac72ed
OS|Linux|0.0.0 Linux 4.18.0-17-generic #18~18.04.1-Ubuntu SMP Fri Mar 15 15:27:12 UTC 2019 x86_64
CPU|amd64|family 6 model 94 stepping 3|1
GPU|||
Crash|SIGSEGV /SEGV_MAPERR|0x0|0
0|0|libxul.so|mozilla::dom::IDBFactory::OpenInternal(JSContext*, nsIPrincipal*, nsTSubstring<char16_t> const&, mozilla::dom::Optional<unsigned long> const&, mozilla::dom::Optional<mozilla::dom::StorageType> const&, bool, mozilla::dom::CallerType, mozilla::ErrorResult&)|hg:hg.mozilla.org/mozilla-central:dom/indexedDB/IDBFactory.cpp:98b223de054374a3fba8669750eed1a8b3247baa|579|0x0
0|1|libxul.so|mozilla::dom::IDBFactory::DeleteDatabase(JSContext*, nsTSubstring<char16_t> const&, mozilla::dom::IDBOpenDBOptions const&, mozilla::dom::CallerType, mozilla::ErrorResult&)|hg:hg.mozilla.org/mozilla-central:dom/indexedDB/IDBFactory.cpp:98b223de054374a3fba8669750eed1a8b3247baa|493|0x5
0|2|libxul.so|mozilla::dom::IDBFactory_Binding::deleteDatabase|s3:gecko-generated-sources:82d801f0158ca76b6a5f73ed1004206c089c5c6a4070c26dbc7ce892615c6ec98472fffdfe21bbdec350687635fa656c05ea2c9fded7ca5b45a29d333f4585ea/dom/bindings/IDBFactoryBinding.cpp:|362|0x3b
0|3|libxul.so|bool mozilla::dom::binding_detail::GenericMethod<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:98b223de054374a3fba8669750eed1a8b3247baa|3150|0x9
0|4|libxul.so|CallJSNative(JSContext*, bool ()(JSContext, unsigned int, JS::Value*), JS::CallArgs const&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:98b223de054374a3fba8669750eed1a8b3247baa|442|0x6
0|5|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:98b223de054374a3fba8669750eed1a8b3247baa|534|0x12
0|6|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:98b223de054374a3fba8669750eed1a8b3247baa|589|0xd
0|7|libxul.so|js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/jit/BaselineIC.cpp:98b223de054374a3fba8669750eed1a8b3247baa|3876|0x13
0|8|||||0x2855caf4d633
0|9|||||0x7f32394d9588
0|10|||||0x2855caf44ac4
0|11|libxul.so|js::jit::EnterBaselineAtBranch(JSContext*, js::InterpreterFrame*, unsigned char*)|hg:hg.mozilla.org/mozilla-central:js/src/jit/BaselineJIT.cpp:98b223de054374a3fba8669750eed1a8b3247baa|113|0x17
0|12|libxul.so|Interpret|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:98b223de054374a3fba8669750eed1a8b3247baa|1982|0x10
0|13|libxul.so|js::RunScript(JSContext*, js::RunState&)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:98b223de054374a3fba8669750eed1a8b3247baa|422|0xb
0|14|libxul.so|js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:98b223de054374a3fba8669750eed1a8b3247baa|562|0xf
0|15|libxul.so|InternalCall|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:98b223de054374a3fba8669750eed1a8b3247baa|589|0xd
0|16|libxul.so|js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>)|hg:hg.mozilla.org/mozilla-central:js/src/vm/Interpreter.cpp:98b223de054374a3fba8669750eed1a8b3247baa|605|0x5
0|17|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:98b223de054374a3fba8669750eed1a8b3247baa|2621|0x1c
0|18|libxul.so|mozilla::dom::EventListener::HandleEvent(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, mozilla::ErrorResult&)|s3:gecko-generated-sources:e009e0a620f59be9f7222e1a55363534d06e5c1dbc04f6806a7e22fdd1b3605bc718f84fa3d329b26bd6e80e748ec27e8716e82c4ac608b3311299526e72dde5/dom/bindings/EventListenerBinding.cpp:|52|0x5
0|19|libxul.so|void mozilla::dom::EventListener::HandleEvent<mozilla::dom::EventTarget*>(mozilla::dom::EventTarget* const&, mozilla::dom::Event&, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JS::Realm*)|s3:gecko-generated-sources:f3d9c01258576daaac3afc4fb3b283652e7f1168abb5287eff6775451ebd0ab6a0e4c8d88d3a67f7147042501bc091c6dfed25b4b8ccf4e4f420897b8d0ba906/dist/include/mozilla/dom/EventListenerBinding.h:|66|0x1c
0|20|libxul.so|mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, mozilla::dom::Event*, mozilla::dom::EventTarget*)|hg:hg.mozilla.org/mozilla-central:dom/events/EventListenerManager.cpp:98b223de054374a3fba8669750eed1a8b3247baa|1040|0x1e
0|21|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:98b223de054374a3fba8669750eed1a8b3247baa|1240|0x19
0|22|libxul.so|mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor&, mozilla::ELMCreationDetector&)|hg:hg.mozilla.org/mozilla-central:dom/events/EventListenerManager.h:98b223de054374a3fba8669750eed1a8b3247baa|356|0x6
0|23|libxul.so|mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)|hg:hg.mozilla.org/mozilla-central:dom/events/EventDispatcher.cpp:98b223de054374a3fba8669750eed1a8b3247baa|551|0x12
0|24|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:98b223de054374a3fba8669750eed1a8b3247baa|1047|0x1a
0|25|libxul.so|nsDocumentViewer::LoadComplete(nsresult)|hg:hg.mozilla.org/mozilla-central:layout/base/nsDocumentViewer.cpp:98b223de054374a3fba8669750eed1a8b3247baa|1098|0x25
0|26|libxul.so|nsDocShell::EndPageLoad(nsIWebProgress, nsIChannel*, nsresult)|hg:hg.mozilla.org/mozilla-central:docshell/base/nsDocShell.cpp:98b223de054374a3fba8669750eed1a8b3247baa|6594|0x18
0|27|libxul.so|nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult)|hg:hg.mozilla.org/mozilla-central:docshell/base/nsDocShell.cpp:98b223de054374a3fba8669750eed1a8b3247baa|6395|0x18
0|28|libxul.so|nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:98b223de054374a3fba8669750eed1a8b3247baa|1313|0x2b
0|29|libxul.so|nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:98b223de054374a3fba8669750eed1a8b3247baa|872|0x22
0|30|libxul.so|nsDocLoader::DocLoaderIsEmpty(bool)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:98b223de054374a3fba8669750eed1a8b3247baa|710|0x15
0|31|libxul.so|nsDocLoader::OnStopRequest(nsIRequest*, nsresult)|hg:hg.mozilla.org/mozilla-central:uriloader/base/nsDocLoader.cpp:98b223de054374a3fba8669750eed1a8b3247baa|598|0x16
0|32|libxul.so|mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult)|hg:hg.mozilla.org/mozilla-central:netwerk/base/nsLoadGroup.cpp:98b223de054374a3fba8669750eed1a8b3247baa|568|0x17
0|33|libxul.so|mozilla::dom::Document::DoUnblockOnload()|hg:hg.mozilla.org/mozilla-central:dom/base/Document.cpp:98b223de054374a3fba8669750eed1a8b3247baa|7831|0x20
0|34|libxul.so|mozilla::dom::Document::UnblockOnload(bool)|hg:hg.mozilla.org/mozilla-central:dom/base/Document.cpp:98b223de054374a3fba8669750eed1a8b3247baa|7763|0x8
0|35|libxul.so|mozilla::dom::Document::DispatchContentLoadedEvents()|hg:hg.mozilla.org/mozilla-central:dom/base/Document.cpp:98b223de054374a3fba8669750eed1a8b3247baa|4891|0xd
0|36|libxul.so|mozilla::detail::RunnableMethodImpl<mozilla::dom::Document*, void (mozilla::dom::Document::)(), true, (mozilla::RunnableKind)0>::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.h:98b223de054374a3fba8669750eed1a8b3247baa|1122|0x13
0|37|libxul.so|mozilla::SchedulerGroup::Runnable::Run()|hg:hg.mozilla.org/mozilla-central:xpcom/threads/SchedulerGroup.cpp:98b223de054374a3fba8669750eed1a8b3247baa|295|0x15
0|38|libxul.so|nsThread::ProcessNextEvent(bool, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThread.cpp:98b223de054374a3fba8669750eed1a8b3247baa|1180|0x15
0|39|libxul.so|NS_ProcessNextEvent(nsIThread*, bool)|hg:hg.mozilla.org/mozilla-central:xpcom/threads/nsThreadUtils.cpp:98b223de054374a3fba8669750eed1a8b3247baa|486|0x11
0|40|libxul.so|mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:98b223de054374a3fba8669750eed1a8b3247baa|88|0xa
0|41|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:98b223de054374a3fba8669750eed1a8b3247baa|315|0x17
0|42|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:98b223de054374a3fba8669750eed1a8b3247baa|308|0x8
0|43|libxul.so|nsBaseAppShell::Run()|hg:hg.mozilla.org/mozilla-central:widget/nsBaseAppShell.cpp:98b223de054374a3fba8669750eed1a8b3247baa|137|0xd
0|44|libxul.so|XRE_RunAppShell()|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:98b223de054374a3fba8669750eed1a8b3247baa|919|0x11
0|45|libxul.so|mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*)|hg:hg.mozilla.org/mozilla-central:ipc/glue/MessagePump.cpp:98b223de054374a3fba8669750eed1a8b3247baa|238|0x5
0|46|libxul.so|MessageLoop::RunInternal()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:98b223de054374a3fba8669750eed1a8b3247baa|315|0x17
0|47|libxul.so|MessageLoop::Run()|hg:hg.mozilla.org/mozilla-central:ipc/chromium/src/base/message_loop.cc:98b223de054374a3fba8669750eed1a8b3247baa|308|0x8
0|48|libxul.so|XRE_InitChildProcess(int, char**, XREChildData const*)|hg:hg.mozilla.org/mozilla-central:toolkit/xre/nsEmbedFunctions.cpp:98b223de054374a3fba8669750eed1a8b3247baa|757|0xc
0|49|firefox-bin|content_process_main(mozilla::Bootstrap*, int, char**)|hg:hg.mozilla.org/mozilla-central:ipc/contentproc/plugin-container.cpp:98b223de054374a3fba8669750eed1a8b3247baa|56|0x14
0|50|firefox-bin|main|hg:hg.mozilla.org/mozilla-central:browser/app/nsBrowserApp.cpp:98b223de054374a3fba8669750eed1a8b3247baa|263|0x11
0|51|libc-2.27.so|__libc_start_main|||0xe7
0|52|firefox-bin|_start|||0x29

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:overholt, could you have a look please?

Comment 2

[Andrew Overholt [:overholt]]

We'll get to this soon. Thanks for the testcase, Jason!

Comment 3

[Tyson Smith [:tsmith]]

Attachment: testcase.html

Updated test case.

Comment 4

[Tyson Smith [:tsmith]]

A Pernosco session is available here: https://pernos.co/debug/7x94fnxxrT5xg8vfG6Pu8A/index.html

Comment 5

[Simon Giesecke [:sg] [he/him]]

(In reply to Tyson Smith [:tsmith] from comment #4)

A Pernosco session is available here: https://pernos.co/debug/7x94fnxxrT5xg8vfG6Pu8A/index.html

Thanks, this is very helpful!

I am not sure what the expected behaviour here is.

Currently, removing all the child nodes leads to the DocShell being destroyed, and this calls https://searchfox.org/mozilla-central/rev/d24696b5abaf9fb75f7985952eab50d5f4ed52ac/dom/indexedDB/IDBFactory.cpp#799, which sets mGlobal to nullptr which eventually triggers the assertion in the subsequent call to IDBFactory.open.

One way to remedy this is to replace the assertion at https://searchfox.org/mozilla-central/rev/d24696b5abaf9fb75f7985952eab50d5f4ed52ac/dom/indexedDB/IDBFactory.cpp#563 to

    aRv.Throw(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
    return nullptr;
  }

Or should the IDBFactory.open call actually succeed? Then the destruction of the DocShell must be prevented, which is not under control of IndexedDB.

Comment 6

[Jan Varga [:janv]]

Yeah, but it would be probably better to keep the assertion and add checks to the methods which call OpenInternal. IDBFactory::Open tries to warn if storage options was passed, and mGlobal is needed for that (for getting the window), but I think it would be cleaner to catch null mGlobal very early.

Comment 7

[Simon Giesecke [:sg] [he/him]]

(In reply to Jan Varga [:janv] from comment #6)

Yeah, but it would be probably better to keep the assertion and add checks to the methods which call OpenInternal. IDBFactory::Open tries to warn if storage options was passed, and mGlobal is needed for that (for getting the window), but I think it would be cleaner to catch null mGlobal very early.

The code checking if storage options were passed is going to be removed, since the custom IDBFactory.open overload is going to be removed.

I don't understand what you mean by "very early". I don't think it is a good idea to duplicate the check at the various call sites of OpenInternal, when it could be done in OpenInternal in the same way for all callers. This might open up to a missing check in some call site, or accidental variations in handling it.

Comment 8

[Jan Varga [:janv]]

When is it going to be removed (approximately)?

Comment 9

[Jan Varga [:janv]]

If this going to be removed rather soon, then ok, change the assertion to a check.

Comment 10

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1543154 - Throw an exception instead of asserting in case there is no mGlobal on a call to IDBFactory.open. r=#dom-workers-and-storage

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1dd6fbf5857a
Throw an exception instead of asserting in case there is no mGlobal on a call to IDBFactory.open. r=dom-workers-and-storage-reviewers,janv

Comment 12

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/1dd6fbf5857a

Comment 13

[Ryan VanderMeulen [:RyanVM]]

Can we land a test for this?

Comment 14

[Simon Giesecke [:sg] [he/him]]

(In reply to Ryan VanderMeulen [:RyanVM][PTO Jan 25 - Feb 2] from comment #13)

Can we land a test for this?

Yes, thanks for the reminder. I will create one based on the attached test case.

Comment 15

[Simon Giesecke [:sg] [he/him]]

Attachment: Bug 1543154 - Added test case from fuzzer. r=#dom-workers-and-storage

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e165680524af
Added test case from fuzzer. r=dom-workers-and-storage-reviewers,edenchuang

Comment 17

[Sandor Molnar[:smolnar]]

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