From f9fe116fa931014468a8f7b954203c1845b980c2 Mon Sep 17 00:00:00 2001 From: Cameron Kaiser Date: Wed, 18 Jul 2018 19:39:47 -0700 Subject: [PATCH] #512: M1472018 M1469309 M1472925 M1470260 (part 1) --- dom/crypto/WebCryptoThreadPool.cpp | 21 ++++++++++++++++++--- dom/crypto/WebCryptoThreadPool.h | 2 ++ dom/media/GraphDriver.cpp | 12 ++++++------ dom/media/GraphDriver.h | 5 ++--- dom/media/MediaStreamGraph.cpp | 3 ++- hal/Hal.cpp | 1 + hal/HalSensor.h | 3 +-- hal/sandbox/SandboxHal.cpp | 6 +++--- layout/base/nsRefreshDriver.cpp | 4 ++++ 9 files changed, 39 insertions(+), 18 deletions(-) diff --git a/dom/crypto/WebCryptoThreadPool.cpp b/dom/crypto/WebCryptoThreadPool.cpp index 3abe347dd..58350e156 100644 --- a/dom/crypto/WebCryptoThreadPool.cpp +++ b/dom/crypto/WebCryptoThreadPool.cpp @@ -64,6 +64,10 @@ WebCryptoThreadPool::DispatchInternal(nsIRunnable* aRunnable) { MutexAutoLock lock(mMutex); + if (mShutdown) { + return NS_ERROR_FAILURE; + } + if (!mPool) { nsCOMPtr pool(do_CreateInstance(NS_THREADPOOL_CONTRACTID)); NS_ENSURE_TRUE(pool, NS_ERROR_FAILURE); @@ -81,10 +85,21 @@ void WebCryptoThreadPool::Shutdown() { MOZ_ASSERT(NS_IsMainThread(), "Wrong thread!"); - MutexAutoLock lock(mMutex); - if (mPool) { - mPool->Shutdown(); + // Limit the scope of locking to avoid deadlocking if DispatchInternal ends + // up getting called during shutdown event processing. + nsCOMPtr pool; + { + MutexAutoLock lock(mMutex); + if (mShutdown) { + return; + } + pool = mPool; + mShutdown = true; + } + + if (pool) { + pool->Shutdown(); } nsCOMPtr obs = mozilla::services::GetObserverService(); diff --git a/dom/crypto/WebCryptoThreadPool.h b/dom/crypto/WebCryptoThreadPool.h index e6f0f2fb1..382b4c0fd 100644 --- a/dom/crypto/WebCryptoThreadPool.h +++ b/dom/crypto/WebCryptoThreadPool.h @@ -27,6 +27,7 @@ private: WebCryptoThreadPool() : mMutex("WebCryptoThreadPool::mMutex") , mPool(nullptr) + , mShutdown(false) { } virtual ~WebCryptoThreadPool() { } @@ -46,6 +47,7 @@ private: mozilla::Mutex mMutex; nsCOMPtr mPool; + bool mShutdown; }; } // namespace dom diff --git a/dom/media/GraphDriver.cpp b/dom/media/GraphDriver.cpp index 83f22e9a2..bd1d40bf0 100644 --- a/dom/media/GraphDriver.cpp +++ b/dom/media/GraphDriver.cpp @@ -173,7 +173,7 @@ public: STREAM_LOG(LogLevel::Debug, ("Starting system thread")); profiler_register_thread("MediaStreamGraph", &aLocal); LIFECYCLE_LOG("Starting a new system driver for graph %p\n", - mDriver->mGraphImpl); + mDriver->mGraphImpl.get()); if (mDriver->mPreviousDriver) { LIFECYCLE_LOG("%p releasing an AudioCallbackDriver(%p), for graph %p\n", mDriver, @@ -204,7 +204,7 @@ private: void ThreadedDriver::Start() { - LIFECYCLE_LOG("Starting thread for a SystemClockDriver %p\n", mGraphImpl); + LIFECYCLE_LOG("Starting thread for a SystemClockDriver %p\n", mGraphImpl.get()); nsCOMPtr event = new MediaStreamGraphInitThreadRunnable(this); // Note: mThread may be null during event->Run() if we pass to NewNamedThread! See AudioInitTask nsresult rv = NS_NewNamedThread("MediaStreamGrph", getter_AddRefs(mThread)); @@ -599,7 +599,7 @@ AudioCallbackDriver::Destroy() void AudioCallbackDriver::Resume() { - STREAM_LOG(LogLevel::Debug, ("Resuming audio threads for MediaStreamGraph %p", mGraphImpl)); + STREAM_LOG(LogLevel::Debug, ("Resuming audio threads for MediaStreamGraph %p", mGraphImpl.get())); if (cubeb_stream_start(mAudioStream) != CUBEB_OK) { NS_WARNING("Could not start cubeb stream for MSG."); } @@ -611,12 +611,12 @@ AudioCallbackDriver::Start() // If this is running on the main thread, we can't open the stream directly, // because it is a blocking operation. if (NS_IsMainThread()) { - STREAM_LOG(LogLevel::Debug, ("Starting audio threads for MediaStreamGraph %p from a new thread.", mGraphImpl)); + STREAM_LOG(LogLevel::Debug, ("Starting audio threads for MediaStreamGraph %p from a new thread.", mGraphImpl.get())); RefPtr initEvent = new AsyncCubebTask(this, AsyncCubebOperation::INIT); initEvent->Dispatch(); } else { - STREAM_LOG(LogLevel::Debug, ("Starting audio threads for MediaStreamGraph %p from the previous driver's thread", mGraphImpl)); + STREAM_LOG(LogLevel::Debug, ("Starting audio threads for MediaStreamGraph %p from the previous driver's thread", mGraphImpl.get())); Init(); // Check if we need to resolve promises because the driver just got switched @@ -669,7 +669,7 @@ AudioCallbackDriver::Revive() mGraphImpl->SetCurrentDriver(mNextDriver); mNextDriver->Start(); } else { - STREAM_LOG(LogLevel::Debug, ("Starting audio threads for MediaStreamGraph %p from a new thread.", mGraphImpl)); + STREAM_LOG(LogLevel::Debug, ("Starting audio threads for MediaStreamGraph %p from a new thread.", mGraphImpl.get())); RefPtr initEvent = new AsyncCubebTask(this, AsyncCubebOperation::INIT); initEvent->Dispatch(); diff --git a/dom/media/GraphDriver.h b/dom/media/GraphDriver.h index 597327663..eb13fddaa 100644 --- a/dom/media/GraphDriver.h +++ b/dom/media/GraphDriver.h @@ -183,9 +183,8 @@ protected: GraphTime mIterationStart; // Time of the end of this graph iteration. GraphTime mIterationEnd; - // The MediaStreamGraphImpl that owns this driver. This has a lifetime longer - // than the driver, and will never be null. - MediaStreamGraphImpl* mGraphImpl; + // The MediaStreamGraphImpl associated with this driver. + const RefPtr mGraphImpl; // This enum specifies the wait state of the driver. enum WaitState { diff --git a/dom/media/MediaStreamGraph.cpp b/dom/media/MediaStreamGraph.cpp index 42438e980..5b042dfa7 100644 --- a/dom/media/MediaStreamGraph.cpp +++ b/dom/media/MediaStreamGraph.cpp @@ -2687,7 +2687,8 @@ MediaStreamGraphImpl::Destroy() // First unregister from memory reporting. UnregisterWeakMemoryReporter(this); - // Clear the self reference which will destroy this instance. + // Clear the self reference which will destroy this instance if all + // associated GraphDrivers are destroyed. mSelfRef = nullptr; } diff --git a/hal/Hal.cpp b/hal/Hal.cpp index 9110abd81..dad25bed7 100644 --- a/hal/Hal.cpp +++ b/hal/Hal.cpp @@ -566,6 +566,7 @@ UnregisterSensorObserver(SensorType aSensor, ISensorObserver *aObserver) { AssertMainThread(); if (!gSensorObservers) { + HAL_ERR("Un-registering a sensor when none have been registered"); return; } diff --git a/hal/HalSensor.h b/hal/HalSensor.h index 551c4271d..5175629c9 100644 --- a/hal/HalSensor.h +++ b/hal/HalSensor.h @@ -18,7 +18,6 @@ namespace hal { * If you add or change any here, do the same in GeckoHalDefines.java. */ enum SensorType { - SENSOR_UNKNOWN = -1, SENSOR_ORIENTATION = 0, SENSOR_ACCELERATION = 1, SENSOR_PROXIMITY = 2, @@ -63,7 +62,7 @@ namespace IPC { struct ParamTraits: public ContiguousEnumSerializer< mozilla::hal::SensorType, - mozilla::hal::SENSOR_UNKNOWN, + mozilla::hal::SENSOR_ORIENTATION, mozilla::hal::NUM_SENSOR_TYPE> { }; diff --git a/hal/sandbox/SandboxHal.cpp b/hal/sandbox/SandboxHal.cpp index 4699cedd3..94ee0a113 100644 --- a/hal/sandbox/SandboxHal.cpp +++ b/hal/sandbox/SandboxHal.cpp @@ -16,6 +16,7 @@ #include "mozilla/dom/battery/Types.h" #include "mozilla/dom/network/Types.h" #include "mozilla/dom/ScreenOrientation.h" +#include "mozilla/EnumeratedRange.h" #include "mozilla/Observer.h" #include "mozilla/unused.h" #include "WindowIdentifier.h" @@ -485,9 +486,8 @@ public: hal::UnregisterBatteryObserver(this); hal::UnregisterNetworkObserver(this); hal::UnregisterScreenConfigurationObserver(this); - for (int32_t sensor = SENSOR_UNKNOWN + 1; - sensor < NUM_SENSOR_TYPE; ++sensor) { - hal::UnregisterSensorObserver(SensorType(sensor), this); + for (auto sensor : MakeEnumeratedRange(NUM_SENSOR_TYPE)) { + hal::UnregisterSensorObserver(sensor, this); } hal::UnregisterWakeLockObserver(this); hal::UnregisterSystemClockChangeObserver(this); diff --git a/layout/base/nsRefreshDriver.cpp b/layout/base/nsRefreshDriver.cpp index cc08b66e5..ac5a414a2 100644 --- a/layout/base/nsRefreshDriver.cpp +++ b/layout/base/nsRefreshDriver.cpp @@ -392,6 +392,9 @@ private: virtual bool NotifyVsync(TimeStamp aVsyncTimestamp) override { + // IMPORTANT: All paths through this method MUST hold a strong ref on + // |this| for the duration of the TickRefreshDriver callback. + if (!NS_IsMainThread()) { MOZ_ASSERT(XRE_IsParentProcess()); // Compress vsync notifications such that only 1 may run at a time @@ -412,6 +415,7 @@ private: aVsyncTimestamp); NS_DispatchToMainThread(vsyncEvent); } else { + RefPtr kungFuDeathGrip(this); TickRefreshDriver(aVsyncTimestamp); }