From 6b1a2c2b214492b8117c8abd4dcc6a7a3a138da9 Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Tue, 23 Aug 2016 12:47:45 -0400 Subject: [PATCH] Bug 1255737: don't wait more than 15 seconds for an AudioCallbackDriver to exit r=pehrsons Avoids running into the stalled-shutdown killer if the audio driver/OS is stupid MozReview-Commit-ID: 58SUg2Xt37C --- dom/media/MediaStreamGraph.cpp | 48 ++++++++++++++++++++++++++++++++-- dom/media/MediaStreamGraph.h | 2 ++ 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/dom/media/MediaStreamGraph.cpp b/dom/media/MediaStreamGraph.cpp index 09ea5f26f4be..784bd016313e 100644 --- a/dom/media/MediaStreamGraph.cpp +++ b/dom/media/MediaStreamGraph.cpp @@ -1448,11 +1448,14 @@ MediaStreamGraphImpl::ForceShutDown(ShutdownTicket* aShutdownTicket) namespace { -class MediaStreamGraphShutDownRunnable : public Runnable { +class MediaStreamGraphShutDownRunnable : public Runnable + , public nsITimerCallback { public: explicit MediaStreamGraphShutDownRunnable(MediaStreamGraphImpl* aGraph) : mGraph(aGraph) {} + NS_DECL_ISUPPORTS_INHERITED + NS_IMETHOD Run() override { NS_ASSERTION(mGraph->mDetectedNotRunning, @@ -1471,10 +1474,30 @@ public: } #endif + // Avoid waiting forever for a callback driver to shut down + // synchronously. Reports are that some 3rd-party audio drivers + // occasionally hang in shutdown (both for us and Chrome). + mTimer = do_CreateInstance(NS_TIMER_CONTRACTID); + if (!mTimer) { + return NS_ERROR_FAILURE; + } + mTimer->InitWithCallback(this, + MediaStreamGraph::AUDIO_CALLBACK_DRIVER_SHUTDOWN_TIMEOUT, + nsITimer::TYPE_ONE_SHOT); + mGraph->mDriver->Shutdown(); // This will wait until it's shutdown since // we'll start tearing down the graph after this // We may be one of several graphs. Drop ticket to eventually unblock shutdown. + NS_ASSERTION(mGraph->mForceShutdownTicket, + "AudioCallbackDriver took too long to shut down and we let shutdown" + " continue - freezing and leaking"); + if (!mGraph->mForceShutdownTicket) { + // The timer fired, so we may be deeper in shutdown now. Block any further + // teardown and just leak, for safety. + return NS_OK; + } + mTimer = nullptr; mGraph->mForceShutdownTicket = nullptr; // We can't block past the final LIFECYCLE_WAITING_FOR_STREAM_DESTRUCTION @@ -1507,10 +1530,30 @@ public: } return NS_OK; } + + NS_IMETHOD Notify(nsITimer* aTimer) override + { + // Sigh, driver took too long to shut down. Stop blocking system + // shutdown and hope all is well. Shutdown of this graph will proceed + // if the driver eventually comes back. + NS_ASSERTION(!(mGraph->mForceShutdownTicket), + "AudioCallbackDriver took too long to shut down - probably hung"); + + mGraph->mForceShutdownTicket = nullptr; + return NS_OK; + } + private: + ~MediaStreamGraphShutDownRunnable() {} + + nsCOMPtr mTimer; RefPtr mGraph; }; +NS_IMPL_ISUPPORTS_INHERITED(MediaStreamGraphShutDownRunnable, Runnable, nsITimerCallback) + + + class MediaStreamGraphStableStateRunnable : public Runnable { public: explicit MediaStreamGraphStableStateRunnable(MediaStreamGraphImpl* aGraph, @@ -3263,7 +3306,8 @@ MediaStreamGraph::GetInstance(MediaStreamGraph::GraphDriverType aGraphDriverRequ public: Blocker() : media::ShutdownBlocker(NS_LITERAL_STRING( - "MediaStreamGraph shutdown: blocking on msg thread")) {} + "MediaStreamGraph shutdown: blocking on msg thread")) + {} NS_IMETHOD BlockShutdown(nsIAsyncShutdownClient* aProfileBeforeChange) override diff --git a/dom/media/MediaStreamGraph.h b/dom/media/MediaStreamGraph.h index ac0715cc54a1..71ae5467ddee 100644 --- a/dom/media/MediaStreamGraph.h +++ b/dom/media/MediaStreamGraph.h @@ -1252,6 +1252,8 @@ public: SYSTEM_THREAD_DRIVER, OFFLINE_THREAD_DRIVER }; + static const uint32_t AUDIO_CALLBACK_DRIVER_SHUTDOWN_TIMEOUT = 15*1000; + // Main thread only static MediaStreamGraph* GetInstance(GraphDriverType aGraphDriverRequested, dom::AudioChannel aChannel);