From 48222ef21a7fd62e9c6718b0090a2702dd49398e Mon Sep 17 00:00:00 2001
From: Andreas Pehrson <apehrson@mozilla.com>
Date: Mon, 2 Sep 2019 14:57:47 +0000
Subject: [PATCH] Bug 1577537 - Catch AppendMessage calls that could be too
 late. r=karlt

If all streams and all ports have been destroyed, there's no guarantee that the
graph is still alive. By forbidding AppendMessage calls after this point, we can
catch bugs with the offending callsite still being in the stack.

Differential Revision: https://phabricator.services.mozilla.com/D44223

--HG--
extra : moz-landing-system : lando
---
 dom/media/MediaStreamGraph.cpp   | 18 +++++++++++++-----
 dom/media/MediaStreamGraphImpl.h |  9 +++++++++
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/dom/media/MediaStreamGraph.cpp b/dom/media/MediaStreamGraph.cpp
index 45fb54d51025..9d26a5078244 100644
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -1793,8 +1793,8 @@ void MediaStreamGraphImpl::SignalMainThreadCleanup() {
 
 void MediaStreamGraphImpl::AppendMessage(UniquePtr<ControlMessage> aMessage) {
   MOZ_ASSERT(NS_IsMainThread(), "main thread only");
-  MOZ_ASSERT(!aMessage->GetStream() || !aMessage->GetStream()->IsDestroyed(),
-             "Stream already destroyed");
+  MOZ_ASSERT_IF(aMessage->GetStream(), !aMessage->GetStream()->IsDestroyed());
+  MOZ_DIAGNOSTIC_ASSERT(mMainThreadStreamCount > 0 || mMainThreadPortCount > 0);
 
   if (mDetectedNotRunning &&
       LifecycleStateRef() > LIFECYCLE_WAITING_FOR_MAIN_THREAD_CLEANUP) {
@@ -2001,8 +2001,11 @@ void MediaStream::Destroy() {
     }
     void RunDuringShutdown() override { Run(); }
   };
-  GraphImpl()->RemoveStream(this);
-  GraphImpl()->AppendMessage(MakeUnique<Message>(this));
+  // Keep a reference to the graph, since Message might RunDuringShutdown()
+  // synchronously and make GraphImpl() invalid.
+  RefPtr<MediaStreamGraphImpl> graph = GraphImpl();
+  graph->AppendMessage(MakeUnique<Message>(this));
+  graph->RemoveStream(this);
   // Message::RunDuringShutdown may have removed this stream from the graph,
   // but our kungFuDeathGrip above will have kept this stream alive if
   // necessary.
@@ -2984,7 +2987,11 @@ void MediaInputPort::Destroy() {
     void RunDuringShutdown() override { Run(); }
     MediaInputPort* mPort;
   };
-  GraphImpl()->AppendMessage(MakeUnique<Message>(this));
+  // Keep a reference to the graph, since Message might RunDuringShutdown()
+  // synchronously and make GraphImpl() invalid.
+  RefPtr<MediaStreamGraphImpl> graph = GraphImpl();
+  graph->AppendMessage(MakeUnique<Message>(this));
+  --graph->mMainThreadPortCount;
 }
 
 MediaStreamGraphImpl* MediaInputPort::GraphImpl() { return mGraph; }
@@ -3102,6 +3109,7 @@ already_AddRefed<MediaInputPort> ProcessedMediaStream::AllocateInputPort(
     }
   }
   port->SetGraphImpl(GraphImpl());
+  ++GraphImpl()->mMainThreadPortCount;
   GraphImpl()->AppendMessage(MakeUnique<Message>(port));
   return port.forget();
 }
diff --git a/dom/media/MediaStreamGraphImpl.h b/dom/media/MediaStreamGraphImpl.h
index d51aed2cf6f9..82165aa437e8 100644
--- a/dom/media/MediaStreamGraphImpl.h
+++ b/dom/media/MediaStreamGraphImpl.h
@@ -662,6 +662,15 @@ class MediaStreamGraphImpl : public MediaStreamGraph,
    */
   size_t mMainThreadStreamCount = 0;
 
+  /**
+   * Main-thread view of the number of ports in this graph, to catch bugs.
+   *
+   * When this becomes zero, and mMainThreadStreamCount is 0, the graph is
+   * marked as forbidden to add more ControlMessages to. It will be shut down
+   * shortly after.
+   */
+  size_t mMainThreadPortCount = 0;
+
   /**
    * Graphs own owning references to their driver, until shutdown. When a driver
    * switch occur, previous driver is either deleted, or it's ownership is