Bug 1255737 - Don't set a shutdown timer if we don't have a shutdown ticket. r=pehrsons, a=lizzard
authorRandell Jesup <rjesup@jesup.org>
Wed, 24 Aug 2016 12:24:17 -0400
changeset 349943 95e4986d9803655d61e2975b9f06fdaf1937c79f
parent 349942 c0cb39e5174697d693a1e3e006ff429ef7b6bca8
child 349944 3a73ccd4e90e826d14b54d084d0fb3768271796c
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspehrsons, lizzard
bugs1255737
milestone50.0a2
Bug 1255737 - Don't set a shutdown timer if we don't have a shutdown ticket. r=pehrsons, a=lizzard
dom/media/MediaStreamGraph.cpp
dom/media/MediaStreamGraph.h
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -1630,22 +1630,25 @@ MediaStreamGraphImpl::ForceShutDown(Shut
     EnsureNextIterationLocked();
   }
 }
 
 /* static */ StaticRefPtr<nsIAsyncShutdownBlocker> gMediaStreamGraphShutdownBlocker;
 
 namespace {
 
-class MediaStreamGraphShutDownRunnable : public Runnable {
+class MediaStreamGraphShutDownRunnable : public Runnable
+                                       , public nsITimerCallback {
 public:
   explicit MediaStreamGraphShutDownRunnable(MediaStreamGraphImpl* aGraph)
     : mGraph(aGraph)
   {}
-  NS_IMETHOD Run()
+  NS_DECL_ISUPPORTS_INHERITED
+
+  NS_IMETHOD Run() override
   {
     NS_ASSERTION(mGraph->mDetectedNotRunning,
                  "We should know the graph thread control loop isn't running!");
 
     LIFECYCLE_LOG("Shutting down graph %p", mGraph.get());
 
     // We've asserted the graph isn't running.  Use mDriver instead of CurrentDriver
     // to avoid thread-safety checks
@@ -1653,20 +1656,43 @@ public:
     // 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
 
+    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
     // stage, since completion of that stage requires all streams to be freed,
     // which requires shutdown to proceed.
 
     // mGraph's thread is not running so it's OK to do whatever here
     if (mGraph->IsEmpty()) {
@@ -1689,20 +1715,40 @@ public:
         stream->GetStreamTracks().Clear();
       }
 
       mGraph->mLifecycleState =
         MediaStreamGraphImpl::LIFECYCLE_WAITING_FOR_STREAM_DESTRUCTION;
     }
     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<nsITimer> mTimer;
   RefPtr<MediaStreamGraphImpl> mGraph;
 };
 
+NS_IMPL_ISUPPORTS_INHERITED(MediaStreamGraphShutDownRunnable, Runnable, nsITimerCallback)
+
+
+
 class MediaStreamGraphStableStateRunnable : public Runnable {
 public:
   explicit MediaStreamGraphStableStateRunnable(MediaStreamGraphImpl* aGraph,
                                                bool aSourceIsMSG)
     : mGraph(aGraph)
     , mSourceIsMSG(aSourceIsMSG)
   {
   }
@@ -3388,17 +3434,18 @@ MediaStreamGraph::GetInstance(MediaStrea
   if (!gGraphs.Get(channel, &graph)) {
     if (!gMediaStreamGraphShutdownBlocker) {
 
       class Blocker : public media::ShutdownBlocker
       {
       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
         {
           // Distribute the global async shutdown blocker in a ticket. If there
           // are zero graphs then shutdown is unblocked when we go out of scope.
           RefPtr<MediaStreamGraphImpl::ShutdownTicket> ticket =
               new MediaStreamGraphImpl::ShutdownTicket(gMediaStreamGraphShutdownBlocker.get());
--- a/dom/media/MediaStreamGraph.h
+++ b/dom/media/MediaStreamGraph.h
@@ -1221,16 +1221,18 @@ public:
   // fact that they will need an audio stream at some point by passing
   // AUDIO_THREAD_DRIVER when getting an instance of MediaStreamGraph, so that
   // the graph starts with the right driver.
   enum GraphDriverType {
     AUDIO_THREAD_DRIVER,
     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);
   static MediaStreamGraph* CreateNonRealtimeInstance(TrackRate aSampleRate);
   // Idempotent
   static void DestroyNonRealtimeInstance(MediaStreamGraph* aGraph);
 
   virtual nsresult OpenAudioInput(int aID,