From 8b280045f5c6b3279b0a3db69827e7ff47c9684b Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Mon, 21 Sep 2020 19:18:43 +0000 Subject: [PATCH] Bug 1663747: Part 1 - Fix sCurrentShutdownPhase and add PastShutdownPhase() API. r=nika Differential Revision: https://phabricator.services.mozilla.com/D89809 --- dom/ipc/ContentParent.cpp | 8 ++++++++ gfx/vr/VRServiceHost.cpp | 5 ++++- intl/strres/nsStringBundle.cpp | 14 ++++++++++++++ xpcom/base/ClearOnShutdown.cpp | 10 ++++++---- xpcom/base/ClearOnShutdown.h | 7 +++++++ 5 files changed, 39 insertions(+), 5 deletions(-) diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 666af2e96da35..38aa1376d2b4f 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -2565,6 +2565,14 @@ ContentParent::~ContentParent() { } bool ContentParent::InitInternal(ProcessPriority aInitialPriority) { + // We can't access the locale service after shutdown has started. Since we + // can't init the process without it, and since we're going to be canceling + // whatever load attempt that initiated this process creation anyway, just + // bail out now if shutdown has already started. + if (PastShutdownPhase(ShutdownPhase::Shutdown)) { + return false; + } + XPCOMInitData xpcomInit; MOZ_LOG(ContentParent::GetLog(), LogLevel::Debug, diff --git a/gfx/vr/VRServiceHost.cpp b/gfx/vr/VRServiceHost.cpp index 6c39f867fc180..7250bfe74d8ef 100644 --- a/gfx/vr/VRServiceHost.cpp +++ b/gfx/vr/VRServiceHost.cpp @@ -237,7 +237,10 @@ void VRServiceHost::SendPuppetSubmitToVRProcess( } void VRServiceHost::PuppetReset() { - if (!mVRProcessEnabled) { + // If we're already into ShutdownFinal, the VRPuppetCommandBuffer instance + // will have been cleared, so don't try to access it after that point. + if (!mVRProcessEnabled && + !(NS_IsMainThread() && PastShutdownPhase(ShutdownPhase::ShutdownFinal))) { // Puppet is running in this process, tell it to reset directly. VRPuppetCommandBuffer::Get().Reset(); } diff --git a/intl/strres/nsStringBundle.cpp b/intl/strres/nsStringBundle.cpp index f4855bcaf1079..660e9333bf14c 100644 --- a/intl/strres/nsStringBundle.cpp +++ b/intl/strres/nsStringBundle.cpp @@ -26,6 +26,7 @@ #include "nsSimpleEnumerator.h" #include "nsStringStream.h" #include "mozilla/BinarySearch.h" +#include "mozilla/ClearOnShutdown.h" #include "mozilla/ResultExtensions.h" #include "mozilla/URLPreloader.h" #include "mozilla/ResultExtensions.h" @@ -483,6 +484,19 @@ nsresult SharedStringBundle::LoadProperties() { return NS_OK; } + MOZ_ASSERT(NS_IsMainThread(), + "String bundles must be initialized on the main thread " + "before they may be used off-main-thread"); + + // We can't access the locale service after shutdown has started, which + // means we can't attempt to load chrome: locale resources (which most of + // our string bundles come from). Since shared string bundles won't be + // useful after shutdown has started anyway (and we almost certainly got + // here from a pre-load attempt in an idle task), just bail out. + if (PastShutdownPhase(ShutdownPhase::Shutdown)) { + return NS_ERROR_ILLEGAL_DURING_SHUTDOWN; + } + // We should only populate shared memory string bundles in the parent // process. Instances in the child process should always be instantiated // with a shared memory file descriptor sent from the parent. diff --git a/xpcom/base/ClearOnShutdown.cpp b/xpcom/base/ClearOnShutdown.cpp index 33f165839e790..a03dfba044556 100644 --- a/xpcom/base/ClearOnShutdown.cpp +++ b/xpcom/base/ClearOnShutdown.cpp @@ -16,8 +16,7 @@ ShutdownPhase sCurrentShutdownPhase = ShutdownPhase::NotInShutdown; void InsertIntoShutdownList(ShutdownObserver* aObserver, ShutdownPhase aPhase) { // Adding a ClearOnShutdown for a "past" phase is an error. - if (!(static_cast(sCurrentShutdownPhase) < - static_cast(aPhase))) { + if (PastShutdownPhase(aPhase)) { MOZ_ASSERT(false, "ClearOnShutdown for phase that already was cleared"); aObserver->Shutdown(); delete aObserver; @@ -39,8 +38,11 @@ void KillClearOnShutdown(ShutdownPhase aPhase) { MOZ_ASSERT(NS_IsMainThread()); // Shutdown only goes one direction... - MOZ_ASSERT(static_cast(sCurrentShutdownPhase) < - static_cast(aPhase)); + MOZ_ASSERT(!PastShutdownPhase(aPhase)); + + // Set the phase before notifying observers to make sure that they can't run + // any code which isn't allowed to run after the start of this phase. + sCurrentShutdownPhase = aPhase; // It's impossible to add an entry for a "past" phase; this is blocked in // ClearOnShutdown, but clear them out anyways in case there are phases diff --git a/xpcom/base/ClearOnShutdown.h b/xpcom/base/ClearOnShutdown.h index 359972a78b980..522428736c05a 100644 --- a/xpcom/base/ClearOnShutdown.h +++ b/xpcom/base/ClearOnShutdown.h @@ -129,6 +129,13 @@ inline void RunOnShutdown(CallableT&& aCallable, new FunctionInvoker(std::forward(aCallable)), aPhase); } +inline bool PastShutdownPhase(ShutdownPhase aPhase) { + MOZ_ASSERT(NS_IsMainThread()); + + return size_t(ClearOnShutdown_Internal::sCurrentShutdownPhase) >= + size_t(aPhase); +} + // Called when XPCOM is shutting down, after all shutdown notifications have // been sent and after all threads' event loops have been purged. void KillClearOnShutdown(ShutdownPhase aPhase);