This bug was filed from the Socorro interface and is report bp-34874eca-430c-456d-bfa1-328720181028. ============================================================= Top 10 frames of crashing thread: 0 ntdll.dll KiFastSystemCallRet 1 ntdll.dll ZwWaitForKeyedEvent 2 ntdll.dll RtlpWaitCouldDeadlock 3 mozglue.dll mozilla::detail::MutexImpl::lock mozglue/misc/Mutex_windows.cpp:27 4 xul.dll static nsresult `anonymous namespace'::DataStorageSharedThread::Initialize security/manager/ssl/DataStorage.cpp:86 5 xul.dll mozilla::DataStorage::Init security/manager/ssl/DataStorage.cpp:379 6 xul.dll mozilla::DataStorage::GetAllChildProcessData security/manager/ssl/DataStorage.cpp:286 7 xul.dll void mozilla::dom::ContentParent::InitInternal dom/ipc/ContentParent.cpp:2487 8 xul.dll bool mozilla::dom::ContentParent::LaunchSubprocess dom/ipc/ContentParent.cpp:2274 9 xul.dll mozilla::dom::ContentParent::GetNewOrUsedBrowserProcess dom/ipc/ContentParent.cpp:888 ============================================================= shutdownhangs with this signature or similar start to show up with firefox 63 on windows in low-to-mid volume, likely related to the changes from bug 1470918.

Judging by some stack traces we've received in crash reports, while shutting down the DataStorageSharedThread, it is possible to process an event on that thread that causes an attempt to re-initialize DataStorage. This wouldn't be a problem because we have a shutdown sentinel boolean and we exit early if it is true. However, checking the boolean involves acquiring the static lock for the thread, which means we can't be holding the lock while we're shutting down the thread.

Pushed by dkeeler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c3dd82a036ef fix a deadlock by not holding the DataStorageSharedThread lock while shutting the thread down r=jcj

Given the crash reports we've seen so far, I don't expect we're going to get any validation of this fix's effectiveness until it makes it to Beta. It grafts cleanly, so please nominate for approval when you get a chance.

We shouldn't reenter DataStorageSharedThread::Shutdown(), but it may be possible. To guard against potentially attempting to shut down the shared thread more than once, we can check gDataStorageSharedThreadShutDown first.

I noticed a potential issue with the first patch, so let's fix that and then uplift.

Pushed by dkeeler@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/f38aa27974ee follow-up to ensure reentering DataStorageSharedThread::Shutdown() doesn't cause problems r=jcj

is everything in place for an uplift to beta now?

Comment on attachment 9024601 [details] bug 1502841 - fix a deadlock by not holding the DataStorageSharedThread lock while shutting the thread down r?jcj [Beta/Release Uplift Approval Request] Feature/Bug causing the regression: Bug 1470918 User impact if declined: Potential shutdown hangs Is this code covered by automated tests?: Yes Has the fix been verified in Nightly?: Yes Needs manual test from QE?: No If yes, steps to reproduce: List of other uplifts needed: the other patch in this bug Risk to taking this patch: Low Why is the change risky/not risky? (and alternatives if risky): The fix is small, localized, and relatively conservative. String changes made/needed: none

Comment on attachment 9025427 [details] bug 1502841 - follow-up to ensure reentering DataStorageSharedThread::Shutdown() doesn't cause problems r?jcj (see comment 11)

Comment on attachment 9024601 [details] bug 1502841 - fix a deadlock by not holding the DataStorageSharedThread lock while shutting the thread down r?jcj PSM deadlock fix, approved for 64.0b14

https://hg.mozilla.org/releases/mozilla-beta/rev/ce690bc50558 https://hg.mozilla.org/releases/mozilla-beta/rev/140ce47ccf2b