From fd2b3d5eb87774f15e9fe4c809db9c58ccf165bc Mon Sep 17 00:00:00 2001 From: Randell Jesup Date: Wed, 24 Aug 2016 12:24:17 -0400 Subject: [PATCH] Bug 1255737: don't set a shutdown timer if we don't have a shutdown ticket r=pehrsons --- dom/media/MediaStreamGraph.cpp | 51 ++++++++++++++++++++++++++++++++-- dom/media/MediaStreamGraph.h | 2 ++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/dom/media/MediaStreamGraph.cpp b/dom/media/MediaStreamGraph.cpp index 44ce45ad3a21..0ae84279efef 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,33 @@ public: } #endif + if (mGraph->mForceShutdownTicket) { + // 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. + if (mTimer && !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 + // 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 +1533,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 +3309,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..689d88e97757 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 = 20*1000; + // Main thread only static MediaStreamGraph* GetInstance(GraphDriverType aGraphDriverRequested, dom::AudioChannel aChannel);