1794591 - Crash in [@ mozilla::dom::indexedDB::(anonymous namespace)::ConnectionPool::CloseConnectionRunnable::Run]

Status: NEW

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

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/d46f20f5-d269-4200-ad83-70ed50221011

MOZ_CRASH Reason: MOZ_DIAGNOSTIC_ASSERT(false) (NS_SUCCEEDED(owningThread->Dispatch(this, nsIEventTarget::DISPATCH_NORMAL)))

Top 10 frames of crashing thread:

0 xul.dll mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:8749
1 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1199
2 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:465
3 xul.dll mozilla::dom::indexedDB:: dom/indexedDB/ActorsParent.cpp:8914
4 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1199
5 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:300
6 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:374
7 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:356
8 xul.dll nsThread::ThreadFunc xpcom/threads/nsThread.cpp:384
9 nss3.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:399

We're hitting a diagnostic assertion because it's to late to post a runnable, the ShutdownProgress crash annotation has reached the xpcom-shutdown-threads stage.

I wonder if the solution here would be to remove the MOZ_ALWAYS_SUCCEEDS() call or at least make it conditional on shutdown. Not knowing this code well I don't know if we should stop posting runnables sooner during shutdown (maybe even blocking shutdown until we're done).

Comment 1

[Jens Stutte [:jstutte]]

IIUC we shut down the connection pool during QuotaClient::FinalizeShutdown so it seems weird that we have connections shutting down during xpcom-shutdown-threads.

But looking at ConnectionPool::CloseIdleDatabases I see nothing that makes us wait until the DB is actually shut down, we just dispatch an event and go on. And I read in ConnectionPool::Shutdown:

  CloseIdleDatabases();

  ShutdownIdleThreads();

so we seem to make no attempt to close or wait for non-idle database connections, neither.

I think we see basically something very similar to bug 1791767 here (or just another manifestation of the same underlying problem).

Comment 2

[Jens Stutte [:jstutte]]

Frequency is low enough (and we have only a diagnostic assert here) to downgrade the severity on this bug. My assumption is that without the diagnostic assert we just end up hanging like in bug 1791767.

Still we should think about a refactoring of our ConnectionPool on the lines of bug 1791767 comment 2. Jari, want to take a look as side-project?

Comment 3

[Jari Jalkanen]

Yes, we should refactor our ConnectionPool.