bug 1406830 store the AsyncCubeTask SharedThreadPool reference on the AudioCallbackDriver r?padenot
The first AsyncCubebTask dispatch from AudioCallbackDriver::Start() may either
be from MediaStreamGraphImpl::RunInStableState() on the main thread or
ThreadedDriver::RunThread() on a threaded driver thread.
These could potentially occur concurrently when there are multiple
MediaStreamGraphs.
This change removes the race around setting sThreadPool.
SharedThreadPool::Get() would have returned the same pointer, and so
that race was probably mostly benign apart from the potential to add an
extra reference and so hang on shutdown in SharedThreadPool::SpinUntilEmpty().
Storing the reference to the SharedThreadPool on the object using it is the
typical way to use SharedThreadPool. It lets the thread pool be released when
not in use, and lets SharedThreadPool deal with multi-thread access and
shutdown.
MozReview-Commit-ID: 8WutVsAMfJo
--- a/dom/media/GraphDriver.cpp
+++ b/dom/media/GraphDriver.cpp
@@ -21,18 +21,16 @@
extern mozilla::LazyLogModule gMediaStreamGraphLog;
#ifdef LOG
#undef LOG
#endif // LOG
#define LOG(type, msg) MOZ_LOG(gMediaStreamGraphLog, type, msg)
namespace mozilla {
-StaticRefPtr<nsIThreadPool> AsyncCubebTask::sThreadPool;
-
GraphDriver::GraphDriver(MediaStreamGraphImpl* aGraphImpl)
: mIterationStart(0),
mIterationEnd(0),
mGraphImpl(aGraphImpl),
mWaitState(WAITSTATE_RUNNING),
mCurrentTimeStamp(TimeStamp::Now()),
mPreviousDriver(nullptr),
mNextDriver(nullptr),
@@ -466,46 +464,30 @@ AsyncCubebTask::AsyncCubebTask(AudioCall
NS_WARNING_ASSERTION(mDriver->mAudioStream || aOperation == INIT,
"No audio stream!");
}
AsyncCubebTask::~AsyncCubebTask()
{
}
-/* static */
-nsresult
-AsyncCubebTask::EnsureThread()
+SharedThreadPool*
+AudioCallbackDriver::GetInitShutdownThread()
{
- if (!sThreadPool) {
- nsCOMPtr<nsIThreadPool> threadPool =
+ if (!mInitShutdownThread) {
+ mInitShutdownThread =
SharedThreadPool::Get(NS_LITERAL_CSTRING("CubebOperation"), 1);
- sThreadPool = threadPool;
- // Need to null this out before xpcom-shutdown-threads Observers run
- // since we don't know the order that the shutdown-threads observers
- // will run. ClearOnShutdown guarantees it runs first.
- if (!NS_IsMainThread()) {
- nsCOMPtr<nsIRunnable> runnable =
- NS_NewRunnableFunction("AsyncCubebTask::EnsureThread", []() -> void {
- ClearOnShutdown(&sThreadPool, ShutdownPhase::ShutdownThreads);
- });
- AbstractThread::MainThread()->Dispatch(runnable.forget());
- } else {
- ClearOnShutdown(&sThreadPool, ShutdownPhase::ShutdownThreads);
- }
const uint32_t kIdleThreadTimeoutMs = 2000;
- nsresult rv = sThreadPool->SetIdleThreadTimeout(PR_MillisecondsToInterval(kIdleThreadTimeoutMs));
- if (NS_WARN_IF(NS_FAILED(rv))) {
- return rv;
- }
+ mInitShutdownThread->
+ SetIdleThreadTimeout(PR_MillisecondsToInterval(kIdleThreadTimeoutMs));
}
- return NS_OK;
+ return mInitShutdownThread;
}
NS_IMETHODIMP
AsyncCubebTask::Run()
{
MOZ_ASSERT(mDriver);
switch(mOperation) {
--- a/dom/media/GraphDriver.h
+++ b/dom/media/GraphDriver.h
@@ -468,16 +468,21 @@ public:
* mStarted for details. */
bool IsStarted();
/* Tell the driver whether this process is using a microphone or not. This is
* thread safe. */
void SetMicrophoneActive(bool aActive);
void CompleteAudioContextOperations(AsyncCubebOperation aOperation);
+
+ /* Fetch, or create a shared thread pool with up to one thread for
+ * AsyncCubebTask. */
+ SharedThreadPool* GetInitShutdownThread();
+
private:
/**
* On certain MacBookPro, the microphone is located near the left speaker.
* We need to pan the sound output to the right speaker if we are using the
* mic and the built-in speaker, or we will have terrible echo. */
void PanOutputIfNeeded(bool aMicrophoneActive);
/**
* This is called when the output device used by the cubeb stream changes. */
@@ -533,19 +538,19 @@ private:
struct AutoInCallback
{
explicit AutoInCallback(AudioCallbackDriver* aDriver);
~AutoInCallback();
AudioCallbackDriver* mDriver;
};
- /* Thread for off-main-thread initialization and
- * shutdown of the audio stream. */
- nsCOMPtr<nsIThread> mInitShutdownThread;
+ /* Shared thread pool with up to one thread for off-main-thread
+ * initialization and shutdown of the audio stream via AsyncCubebTask. */
+ RefPtr<SharedThreadPool> mInitShutdownThread;
/* This must be accessed with the graph monitor held. */
AutoTArray<StreamAndPromiseForOperation, 1> mPromisesForOperation;
/* Used to queue us to add the mixer callback on first run. */
bool mAddedMixer;
/* This is atomic and is set by the audio callback thread. It can be read by
* any thread safely. */
Atomic<bool> mInCallback;
@@ -561,31 +566,29 @@ private:
class AsyncCubebTask : public Runnable
{
public:
AsyncCubebTask(AudioCallbackDriver* aDriver, AsyncCubebOperation aOperation);
nsresult Dispatch(uint32_t aFlags = NS_DISPATCH_NORMAL)
{
- nsresult rv = EnsureThread();
- if (!NS_FAILED(rv)) {
- rv = sThreadPool->Dispatch(this, aFlags);
+ SharedThreadPool* threadPool = mDriver->GetInitShutdownThread();
+ if (!threadPool) {
+ return NS_ERROR_FAILURE;
}
- return rv;
+ return threadPool->Dispatch(this, aFlags);
}
protected:
virtual ~AsyncCubebTask();
private:
- static nsresult EnsureThread();
+ NS_IMETHOD Run() override final;
- NS_IMETHOD Run() override final;
- static StaticRefPtr<nsIThreadPool> sThreadPool;
RefPtr<AudioCallbackDriver> mDriver;
AsyncCubebOperation mOperation;
RefPtr<MediaStreamGraphImpl> mShutdownGrip;
};
} // namespace mozilla
#endif // GRAPHDRIVER_H_
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -1558,16 +1558,25 @@ public:
if (mGraph->mDriver->AsAudioCallbackDriver()) {
MOZ_ASSERT(!mGraph->mDriver->AsAudioCallbackDriver()->InCallback());
}
#endif
mGraph->mDriver->Shutdown(); // This will wait until it's shutdown since
// we'll start tearing down the graph after this
+ // Release the driver now so that an AudioCallbackDriver will release its
+ // SharedThreadPool reference. Each SharedThreadPool reference must be
+ // released before SharedThreadPool::SpinUntilEmpty() runs on
+ // xpcom-shutdown-threads. Don't wait for GC/CC to release references to
+ // objects owning streams, or for expiration of mGraph->mShutdownTimer,
+ // which won't otherwise release its reference on the graph until
+ // nsTimerImpl::Shutdown(), which runs after xpcom-shutdown-threads.
+ mGraph->mDriver = nullptr;
+
// Safe to access these without the monitor since the graph isn't running.
// We may be one of several graphs. Drop ticket to eventually unblock shutdown.
if (mGraph->mShutdownTimer && !mGraph->mForceShutdownTicket) {
MOZ_ASSERT(false,
"AudioCallbackDriver took too long to shut down and we let shutdown"
" continue - freezing and leaking");
// The timer fired, so we may be deeper in shutdown now. Block any further