Bug 1528279 - Don't record latencies while holding queue lock. r=geckoview-reviewers,snorp
authorAgi Sferro <agi@mozilla.com>
Tue, 16 Apr 2019 22:00:24 +0000
changeset 469735 a1c8daed8d7f
parent 469734 19d9a16423fd
child 469736 f7418820531d
push id35880
push usercbrindusan@mozilla.com
push dateWed, 17 Apr 2019 09:36:19 +0000
treeherdermozilla-central@79e6ed0b08d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgeckoview-reviewers, snorp
bugs1528279
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1528279 - Don't record latencies while holding queue lock. r=geckoview-reviewers,snorp Doing pretty much anything while holding the EventQueue lock is dangerous as we might try to enqueue an event and deadlock. This happens here where nsAppShell::RecordLatencies sometimes tries to enqueue an event to the main thread and deadlocks. To avoid this, we release and reacquire the lock instead. Differential Revision: https://phabricator.services.mozilla.com/D27824
widget/android/nsAppShell.h
--- a/widget/android/nsAppShell.h
+++ b/widget/android/nsAppShell.h
@@ -195,23 +195,33 @@ class nsAppShell : public nsBaseAppShell
         event->mPostTime = Event::GetTime();
         // Ownership of event object transfers to the queue.
         mozilla::Unused << event.release();
       }
       lock.NotifyAll();
     }
 
     mozilla::UniquePtr<Event> Pop(bool mayWait) {
+#ifdef EARLY_BETA_OR_EARLIER
+      bool isQueueEmpty = false;
+      if (mayWait) {
+        mozilla::MonitorAutoLock lock(mMonitor);
+        isQueueEmpty = mQueue.isEmpty();
+      }
+      if (isQueueEmpty) {
+        // Record latencies when we're about to be idle.
+        // Note: We can't call this while holding the lock because
+        // nsAppShell::RecordLatencies may try to dispatch an event to the main
+        // thread which tries to acquire the lock again.
+        nsAppShell::RecordLatencies();
+      }
+#endif
       mozilla::MonitorAutoLock lock(mMonitor);
 
       if (mayWait && mQueue.isEmpty()) {
-#ifdef EARLY_BETA_OR_EARLIER
-        // Record latencies when we're about to be idle.
-        nsAppShell::RecordLatencies();
-#endif
         lock.Wait();
       }
 
       // Ownership of event object transfers to the return value.
       mozilla::UniquePtr<Event> event(mQueue.popFirst());
       if (!event || !event->mPostTime) {
         return event;
       }