Bug 1233812 - Move SyncRunEvent to nsAppShell; r=snorp

This patch moves the SyncRunEvent logic from GLControllerSupport to
nsAppShell, as it could be useful elsewhere. This patch fixes a race
condition related to shutdown, where a deadlock could occur if Gecko
shuts down when another thread is waiting for a synchronous event to
finish running. This patch also fixes a crash on shutdown when we tried
to create a mutex after the deadlock detector has shut down.

widget/android/nsAppShell.cpp

@@ -185,6 +185,8 @@ public:
 };
 
 nsAppShell::nsAppShell()
+    : mSyncRunMonitor("nsAppShell.SyncRun")
+    , mSyncRunQuit(false)
 {
     {
         StaticMutexAutoLock lock(sAppShellLock);
@@ -217,15 +219,15 @@ nsAppShell::nsAppShell()
 
 nsAppShell::~nsAppShell()
 {
-    while (mEventQueue.Pop(/* mayWait */ false)) {
-        NS_WARNING("Discarded event on shutdown");
-    }
-
     {
         StaticMutexAutoLock lock(sAppShellLock);
         sAppShell = nullptr;
     }
 
+    while (mEventQueue.Pop(/* mayWait */ false)) {
+        NS_WARNING("Discarded event on shutdown");
+    }
+
     if (sPowerManagerService) {
         sPowerManagerService->RemoveWakeLockListener(sWakeLockListener);
 
@@ -282,6 +284,12 @@ nsAppShell::Observe(nsISupports* aSubject,
     bool removeObserver = false;
 
     if (!strcmp(aTopic, "xpcom-shutdown")) {
+        {
+            // Release any thread waiting for a sync call to finish.
+            MonitorAutoLock runLock(mSyncRunMonitor);
+            mSyncRunQuit = true;
+            runLock.NotifyAll();
+        }
         // We need to ensure no observers stick around after XPCOM shuts down
         // or we'll see crashes, as the app shell outlives XPConnect.
         mObserversHash.Clear();
@@ -385,6 +393,52 @@ nsAppShell::ProcessNextNativeEvent(bool mayWait)
     return true;
 }
 
+void
+nsAppShell::SyncRunEvent(Event&& event,
+                         UniquePtr<Event>(*eventFactory)(UniquePtr<Event>&&))
+{
+    // Perform the call on the Gecko thread in a separate lambda, and wait
+    // on the monitor on the current thread.
+    MOZ_ASSERT(!NS_IsMainThread());
+
+    // This is the lock to check that app shell is still alive.
+    mozilla::StaticMutexAutoLock shellLock(sAppShellLock);
+    nsAppShell* const appShell = sAppShell;
+
+    if (MOZ_UNLIKELY(!appShell)) {
+        // Post-shutdown.
+        return;
+    }
+
+    // This is the monitor that we will wait on for the call to complete.
+    mozilla::MonitorAutoLock runLock(appShell->mSyncRunMonitor);
+
+    bool finished = false;
+    auto runAndNotify = [&event, &finished] {
+        nsAppShell* const appShell = sAppShell;
+        if (MOZ_UNLIKELY(!appShell || appShell->mSyncRunQuit)) {
+            return;
+        }
+        event.Run();
+        mozilla::MonitorAutoLock runLock(appShell->mSyncRunMonitor);
+        finished = true;
+        runLock.NotifyAll();
+    };
+
+    UniquePtr<Event> runAndNotifyEvent = mozilla::MakeUnique<
+            LambdaEvent<decltype(runAndNotify)>>(mozilla::Move(runAndNotify));
+
+    if (eventFactory) {
+        runAndNotifyEvent = (*eventFactory)(mozilla::Move(runAndNotifyEvent));
+    }
+
+    appShell->mEventQueue.Post(mozilla::Move(runAndNotifyEvent));
+
+    while (!finished && MOZ_LIKELY(!appShell->mSyncRunQuit)) {
+        runLock.Wait();
+    }
+}
+
 class nsAppShell::LegacyGeckoEvent : public Event
 {
     mozilla::UniquePtr<AndroidGeckoEvent> ae;

widget/android/nsAppShell.h

@@ -70,6 +70,27 @@ public:
         void Run() override { return lambda(); }
     };
 
+    class ProxyEvent : public Event
+    {
+    protected:
+        mozilla::UniquePtr<Event> baseEvent;
+
+    public:
+        ProxyEvent(mozilla::UniquePtr<Event>&& event)
+            : baseEvent(mozilla::Move(event))
+        {}
+
+        void PostTo(mozilla::LinkedList<Event>& queue) override
+        {
+            baseEvent->PostTo(queue);
+        }
+
+        void Run() override
+        {
+            baseEvent->Run();
+        }
+    };
+
     static nsAppShell* Get()
     {
         MOZ_ASSERT(NS_IsMainThread());
@@ -113,6 +134,11 @@ public:
 
     static void PostEvent(mozilla::AndroidGeckoEvent* event);
 
+    // Post a event and wait for it to finish running on the Gecko thread.
+    static void SyncRunEvent(Event&& event,
+                             mozilla::UniquePtr<Event>(*eventFactory)(
+                                    mozilla::UniquePtr<Event>&&) = nullptr);
+
     void ResendLastResizeEvent(nsWindow* aDest);
 
     void SetBrowserApp(nsIAndroidBrowserApp* aBrowserApp) {
@@ -189,6 +215,9 @@ protected:
 
     } mEventQueue;
 
+    mozilla::Monitor mSyncRunMonitor;
+    bool mSyncRunQuit;
+
     bool mAllowCoalescingTouches;
 
     nsCOMPtr<nsIAndroidBrowserApp> mBrowserApp;

widget/android/nsWindow.cpp

@@ -381,20 +381,21 @@ class nsWindow::GLControllerSupport final
     GeckoLayerClient::GlobalRef mLayerClient;
     Atomic<bool, ReleaseAcquire> mCompositorPaused;
 
-    class GLControllerEvent final : public nsAppShell::Event
+    // In order to use Event::HasSameTypeAs in PostTo(), we cannot make
+    // GLControllerEvent a template because each template instantiation is
+    // a different type. So implement GLControllerEvent as a ProxyEvent.
+    class GLControllerEvent final : public nsAppShell::ProxyEvent
     {
-        template<class T> using LambdaEvent = nsAppShell::LambdaEvent<T>;
         using Event = nsAppShell::Event;
 
-        // In order to use Event::HasSameTypeAs in PostTo(), we cannot make
-        // GLControllerEvent a template because each template instantiation is
-        // a different type. So encapsulate the lambda inside baseEvent.
-        UniquePtr<Event> baseEvent;
-
     public:
-        template<class T>
-        GLControllerEvent(T* event)
-            : baseEvent(UniquePtr<T, DefaultDelete<Event>>(event))
+        static UniquePtr<Event> MakeEvent(UniquePtr<Event>&& event)
+        {
+            return MakeUnique<GLControllerEvent>(mozilla::Move(event));
+        }
+
+        GLControllerEvent(UniquePtr<Event>&& event)
+            : nsAppShell::ProxyEvent(mozilla::Move(event))
         {}
 
         void PostTo(LinkedList<Event>& queue) override
@@ -411,36 +412,8 @@ class nsWindow::GLControllerSupport final
                 queue.insertBack(this);
             }
         }
-
-        void Run() override
-        {
-            MOZ_ASSERT(baseEvent);
-            baseEvent->Run();
-        }
     };
 
-    static void SyncRunEvent(nsAppShell::Event&& event) {
-        // Create a monitor, perform the call on the Gecko thread in a custom
-        // lambda, and wait on the monitor on the current thread.
-        MOZ_ASSERT(!NS_IsMainThread());
-        Monitor monitor("GLControllerSupport");
-        MonitorAutoLock lock(monitor);
-        bool finished = false;
-        auto callAndNotify = [&event, &monitor, &finished] {
-            event.Run();
-            MonitorAutoLock lock(monitor);
-            finished = true;
-            lock.NotifyAll();
-        };
-        nsAppShell::PostEvent(
-                mozilla::MakeUnique<GLControllerEvent>(
-                new nsAppShell::LambdaEvent<decltype(callAndNotify)>(
-                mozilla::Move(callAndNotify))));
-        while (!finished) {
-            lock.Wait();
-        }
-    }
-
 public:
     typedef GLController::Natives<GLControllerSupport> Base;
 
@@ -453,8 +426,8 @@ public:
             aCall.IsTarget(&GLControllerSupport::PauseCompositor)) {
 
             // These calls are blocking.
-            SyncRunEvent(GeckoViewSupport::WindowEvent<Functor>(
-                    mozilla::Move(aCall)));
+            nsAppShell::SyncRunEvent(GeckoViewSupport::WindowEvent<Functor>(
+                    mozilla::Move(aCall)), &GLControllerEvent::MakeEvent);
             return;
 
         } else if (aCall.IsTarget(
@@ -475,7 +448,7 @@ public:
 
         nsAppShell::PostEvent(
                 mozilla::MakeUnique<GLControllerEvent>(
-                new GeckoViewSupport::WindowEvent<Functor>(
+                mozilla::MakeUnique<GeckoViewSupport::WindowEvent<Functor>>(
                 mozilla::Move(aCall))));
     }