diff --git a/content/media/GraphDriver.cpp b/content/media/GraphDriver.cpp index 4d3818a703dd..5fddd5fdbc7a 100644 --- a/content/media/GraphDriver.cpp +++ b/content/media/GraphDriver.cpp @@ -260,6 +260,7 @@ ThreadedDriver::Stop() if (mThread) { mThread->Shutdown(); + mThread = nullptr; } } @@ -400,19 +401,25 @@ public: NS_IMETHOD Run() { MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(mThread); + mThread->Shutdown(); + mThread = nullptr; return NS_OK; } private: - nsRefPtr mThread; + nsCOMPtr mThread; }; OfflineClockDriver::~OfflineClockDriver() { // transfer the ownership of mThread to the event - nsCOMPtr event = new MediaStreamGraphShutdownThreadRunnable2(mThread); - mThread = nullptr; - NS_DispatchToMainThread(event); + // XXX should use .forget()/etc + if (mThread) { + nsCOMPtr event = new MediaStreamGraphShutdownThreadRunnable2(mThread); + mThread = nullptr; + NS_DispatchToMainThread(event); + } } void @@ -741,19 +748,16 @@ AudioCallbackDriver::DeviceChangedCallback_s(void* aUser) } bool AudioCallbackDriver::InCallback() { - MonitorAutoLock mon(mGraphImpl->GetMonitor()); return mInCallback; } AudioCallbackDriver::AutoInCallback::AutoInCallback(AudioCallbackDriver* aDriver) : mDriver(aDriver) { - MonitorAutoLock mon(mDriver->mGraphImpl->GetMonitor()); mDriver->mInCallback = true; } AudioCallbackDriver::AutoInCallback::~AutoInCallback() { - MonitorAutoLock mon(mDriver->mGraphImpl->GetMonitor()); mDriver->mInCallback = false; } diff --git a/content/media/GraphDriver.h b/content/media/GraphDriver.h index 2a173bf9c209..71d421432431 100644 --- a/content/media/GraphDriver.h +++ b/content/media/GraphDriver.h @@ -11,6 +11,7 @@ #include "AudioBufferUtils.h" #include "AudioMixer.h" #include "AudioSegment.h" +#include "mozilla/Atomics.h" struct cubeb_stream; @@ -198,6 +199,8 @@ public: return mGraphImpl; } + virtual bool OnThread() = 0; + protected: // Time of the start of this graph iteration. GraphTime mIterationStart; @@ -262,6 +265,9 @@ public: uint32_t IterationDuration() { return MEDIA_GRAPH_TARGET_PERIOD_MS; } + + virtual bool OnThread() MOZ_OVERRIDE { return !mThread || NS_GetCurrentThread() == mThread; } + protected: nsCOMPtr mThread; }; @@ -384,6 +390,8 @@ public: */ bool InCallback(); + virtual bool OnThread() MOZ_OVERRIDE { return !mStarted || InCallback(); } + /* Whether the underlying cubeb stream has been started. See comment for * mStarted for details. */ bool IsStarted(); @@ -449,8 +457,7 @@ private: * shutdown of the audio stream. */ nsCOMPtr mInitShutdownThread; dom::AudioChannel mAudioChannel; - /* This can only be accessed with the graph's monitor held. */ - bool mInCallback; + Atomic mInCallback; /* A thread has been created to be able to pause and restart the audio thread, * but has not done so yet. This indicates that the callback should return * early */ diff --git a/content/media/MediaStreamGraph.cpp b/content/media/MediaStreamGraph.cpp index 70284302c3d6..8955f827cd72 100644 --- a/content/media/MediaStreamGraph.cpp +++ b/content/media/MediaStreamGraph.cpp @@ -1486,11 +1486,18 @@ public: LIFECYCLE_LOG("Shutting down graph %p", mGraph.get()); - if (mGraph->CurrentDriver()->AsAudioCallbackDriver()) { - MOZ_ASSERT(!mGraph->CurrentDriver()->AsAudioCallbackDriver()->InCallback()); + // We've asserted the graph isn't running. Use mDriver instead of CurrentDriver + // to avoid thread-safety checks +#if 0 // AudioCallbackDrivers are released asynchronously anyways + // XXX a better test would be have setting mDetectedNotRunning make sure + // any current callback has finished and block future ones -- or just + // handle it all in Shutdown()! + if (mGraph->mDriver->AsAudioCallbackDriver()) { + MOZ_ASSERT(!mGraph->mDriver->AsAudioCallbackDriver()->InCallback()); } +#endif - mGraph->CurrentDriver()->Shutdown(); + mGraph->mDriver->Shutdown(); // mGraph's thread is not running so it's OK to do whatever here if (mGraph->IsEmpty()) { @@ -1720,6 +1727,7 @@ MediaStreamGraphImpl::RunInStableState(bool aSourceIsMSG) #endif } + static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID); void diff --git a/content/media/MediaStreamGraphImpl.h b/content/media/MediaStreamGraphImpl.h index afc408012d2b..f1b6a0690026 100644 --- a/content/media/MediaStreamGraphImpl.h +++ b/content/media/MediaStreamGraphImpl.h @@ -150,6 +150,12 @@ public: void Init(); // The following methods run on the graph thread (or possibly the main thread if // mLifecycleState > LIFECYCLE_RUNNING) + void AssertOnGraphThreadOrNotRunning() { + // either we're on the right thread (and calling CurrentDriver() is safe), + // or we're going to assert anyways, so don't cross-check CurrentDriver + MOZ_ASSERT(mDriver->OnThread() || + (mLifecycleState > LIFECYCLE_RUNNING && NS_IsMainThread())); + } /* * This does the actual iteration: Message processing, MediaStream ordering, * blocking computation and processing. @@ -418,6 +424,12 @@ public: * Not safe to call off the MediaStreamGraph thread unless monitor is held! */ GraphDriver* CurrentDriver() { +#ifdef DEBUG + // #ifdef since we're not wrapping it all in MOZ_ASSERT() + if (!mDriver->OnThread()) { + mMonitor.AssertCurrentThreadOwns(); + } +#endif return mDriver; } @@ -428,6 +440,7 @@ public: * should return and pass the control to the new driver shortly after. */ void SetCurrentDriver(GraphDriver* aDriver) { + MOZ_ASSERT(mDriver->OnThread()); mDriver = aDriver; }