Bug 1501748 - Make GeckoThread.waitOnGecko() time out by default. r=geckoview-reviewers,esawin
authorJames Willcox <snorp@snorp.net>
Wed, 19 Dec 2018 22:59:46 +0000
changeset 509051 9bca860f26dcac8443f9ff59fbce5dd450420990
parent 509050 7ceb0427f883d038bcca141a502845ce6eb92e1b
child 509052 0e9a610fe05f5e830d4cc387b6d293e94347e3c7
push id10547
push userffxbld-merge
push dateMon, 21 Jan 2019 13:03:58 +0000
treeherdermozilla-beta@24ec1916bffe [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgeckoview-reviewers, esawin
bugs1501748
milestone66.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 1501748 - Make GeckoThread.waitOnGecko() time out by default. r=geckoview-reviewers,esawin All of the current usage can survive a timeout, and we'd rather that than a deadlock. Future code that does want to risk a deadlock can call `GeckoThread.waitOnGeckoForever` instead. Differential Revision: https://phabricator.services.mozilla.com/D14560
mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java
widget/android/nsAppShell.cpp
widget/android/nsAppShell.h
--- a/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java
+++ b/mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoThread.java
@@ -1,25 +1,22 @@
 /* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*-
  * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 package org.mozilla.gecko;
 
-import org.mozilla.gecko.TelemetryUtils;
 import org.mozilla.gecko.annotation.RobocopTarget;
 import org.mozilla.gecko.annotation.WrapForJNI;
 import org.mozilla.gecko.mozglue.GeckoLoader;
 import org.mozilla.gecko.process.GeckoProcessManager;
 import org.mozilla.gecko.util.GeckoBundle;
-import org.mozilla.gecko.util.FileUtils;
 import org.mozilla.gecko.util.ThreadUtils;
 import org.mozilla.geckoview.BuildConfig;
-import org.mozilla.geckoview.GeckoRuntimeSettings;
 
 import android.content.Context;
 import android.content.res.Configuration;
 import android.content.res.Resources;
 import android.os.Bundle;
 import android.os.Handler;
 import android.os.Looper;
 import android.os.Message;
@@ -129,16 +126,18 @@ public class GeckoThread extends Thread 
 
     private static TelemetryUtils.Timer sInitTimer;
 
     // Main process parameters
     public static final int FLAG_DEBUGGING = 1 << 0; // Debugging mode.
     public static final int FLAG_PRELOAD_CHILD = 1 << 1; // Preload child during main thread start.
     public static final int FLAG_ENABLE_NATIVE_CRASHREPORTER = 1 << 2; // Enable native crash reporting
 
+    public static final long DEFAULT_TIMEOUT = 5000;
+
     /* package */ static final String EXTRA_ARGS = "args";
     private static final String EXTRA_PREFS_FD = "prefsFd";
     private static final String EXTRA_PREF_MAP_FD = "prefMapFd";
     private static final String EXTRA_IPC_FD = "ipcFd";
     private static final String EXTRA_CRASH_FD = "crashFd";
     private static final String EXTRA_CRASH_ANNOTATION_FD = "crashAnnotationFd";
 
     private boolean mInitialized;
@@ -571,18 +570,31 @@ public class GeckoThread extends Thread 
         // This is almost always called before Gecko loads, so we don't
         // bother checking here if Gecko is actually loaded or not.
         // Speculative connection depends on proxy settings,
         // so the earliest it can happen is after profile is ready.
         queueNativeCallUntil(State.PROFILE_READY, GeckoThread.class,
                              "speculativeConnectNative", uri);
     }
 
-    @WrapForJNI @RobocopTarget
-    public static native void waitOnGecko();
+    @WrapForJNI(stubName = "WaitOnGecko")
+    @RobocopTarget
+    private static native boolean nativeWaitOnGecko(long timeoutMillis);
+
+    public static void waitOnGeckoForever() {
+        nativeWaitOnGecko(0);
+    }
+
+    public static boolean waitOnGecko() {
+        return waitOnGecko(DEFAULT_TIMEOUT);
+    }
+
+    public static boolean waitOnGecko(long timeoutMillis) {
+        return nativeWaitOnGecko(timeoutMillis);
+    }
 
     @WrapForJNI(stubName = "OnPause", dispatchTo = "gecko")
     private static native void nativeOnPause();
 
     public static void onPause() {
         if (isStateAtLeast(State.PROFILE_READY)) {
             nativeOnPause();
         } else {
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -139,33 +139,34 @@ class GeckoThreadSupport final
     }
 
     OriginAttributes attrs;
     nsCOMPtr<nsIPrincipal> principal =
         BasePrincipal::CreateCodebasePrincipal(uri, attrs);
     specConn->SpeculativeConnect2(uri, principal, nullptr);
   }
 
-  static void WaitOnGecko() {
+  static bool WaitOnGecko(int64_t timeoutMillis) {
     struct NoOpRunnable : Runnable {
       NoOpRunnable() : Runnable("NoOpRunnable") {}
       NS_IMETHOD Run() override { return NS_OK; }
     };
 
     struct NoOpEvent : nsAppShell::Event {
       void Run() override {
         // We cannot call NS_DispatchToMainThread from within
         // WaitOnGecko itself because the thread that is calling
         // WaitOnGecko may not be an nsThread, and may not be able to do
         // a sync dispatch.
         NS_DispatchToMainThread(do_AddRef(new NoOpRunnable()),
                                 NS_DISPATCH_SYNC);
       }
     };
-    nsAppShell::SyncRunEvent(NoOpEvent());
+    return nsAppShell::SyncRunEvent(
+        NoOpEvent(), nullptr, TimeDuration::FromMilliseconds(timeoutMillis));
   }
 
   static void OnPause() {
     MOZ_ASSERT(NS_IsMainThread());
 
     sPauseCount++;
     // If sPauseCount is now 1, we just crossed the threshold from "resumed"
     // "paused". so we should notify observers and so on.
@@ -681,57 +682,61 @@ bool nsAppShell::ProcessNextNativeEvent(
   if (!curEvent) return false;
 
   mozilla::BackgroundHangMonitor().NotifyActivity();
 
   curEvent->Run();
   return true;
 }
 
-void nsAppShell::SyncRunEvent(
-    Event&& event, UniquePtr<Event> (*eventFactory)(UniquePtr<Event>&&)) {
+bool nsAppShell::SyncRunEvent(
+    Event&& event, UniquePtr<Event> (*eventFactory)(UniquePtr<Event>&&),
+    const TimeDuration timeout) {
   // 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,
   // and to wait on for the sync call to complete.
   mozilla::MutexAutoLock shellLock(*sAppShellLock);
   nsAppShell* const appShell = sAppShell;
 
   if (MOZ_UNLIKELY(!appShell)) {
     // Post-shutdown.
-    return;
+    return false;
   }
 
   bool finished = false;
   auto runAndNotify = [&event, &finished] {
     nsAppShell* const appShell = nsAppShell::Get();
     if (MOZ_UNLIKELY(!appShell || appShell->mSyncRunQuit)) {
-      return;
+      return false;
     }
     event.Run();
     finished = true;
     mozilla::MutexAutoLock shellLock(*sAppShellLock);
     appShell->mSyncRunFinished.NotifyAll();
+    return finished;
   };
 
   UniquePtr<Event> runAndNotifyEvent =
       mozilla::MakeUnique<LambdaEvent<decltype(runAndNotify)>>(
           std::move(runAndNotify));
 
   if (eventFactory) {
     runAndNotifyEvent = (*eventFactory)(std::move(runAndNotifyEvent));
   }
 
   appShell->mEventQueue.Post(std::move(runAndNotifyEvent));
 
   while (!finished && MOZ_LIKELY(sAppShell && !sAppShell->mSyncRunQuit)) {
-    appShell->mSyncRunFinished.Wait();
+    appShell->mSyncRunFinished.Wait(timeout);
   }
+
+  return finished;
 }
 
 already_AddRefed<nsIURI> nsAppShell::ResolveURI(const nsCString& aUriStr) {
   nsCOMPtr<nsIIOService> ioServ = do_GetIOService();
   nsCOMPtr<nsIURI> uri;
 
   if (NS_SUCCEEDED(
           ioServ->NewURI(aUriStr, nullptr, nullptr, getter_AddRefs(uri)))) {
--- a/widget/android/nsAppShell.h
+++ b/widget/android/nsAppShell.h
@@ -8,16 +8,17 @@
 
 #include <time.h>
 
 #include "mozilla/BackgroundHangMonitor.h"
 #include "mozilla/LinkedList.h"
 #include "mozilla/Monitor.h"
 #include "mozilla/Move.h"
 #include "mozilla/StaticPtr.h"
+#include "mozilla/TimeStamp.h"  // for mozilla::TimeDuration
 #include "mozilla/TypeTraits.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
 #include "mozilla/jni/Natives.h"
 #include "nsBaseAppShell.h"
 #include "nsCOMPtr.h"
 #include "nsTArray.h"
 #include "nsInterfaceHashtable.h"
@@ -61,17 +62,17 @@ class nsAppShell : public nsBaseAppShell
 
   template <typename T>
   class LambdaEvent : public Event {
    protected:
     T lambda;
 
    public:
     explicit LambdaEvent(T&& l) : lambda(std::move(l)) {}
-    void Run() override { return lambda(); }
+    void Run() override { lambda(); }
   };
 
   class ProxyEvent : public Event {
    protected:
     mozilla::UniquePtr<Event> baseEvent;
 
    public:
     explicit ProxyEvent(mozilla::UniquePtr<Event>&& event)
@@ -118,19 +119,21 @@ class nsAppShell : public nsBaseAppShell
     if (!sAppShell) {
       return;
     }
     sAppShell->mEventQueue.Post(
         mozilla::MakeUnique<LambdaEvent<T>>(std::move(lambda)));
   }
 
   // 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);
+  static bool SyncRunEvent(
+      Event&& event,
+      mozilla::UniquePtr<Event> (*eventFactory)(mozilla::UniquePtr<Event>&&) =
+          nullptr,
+      const mozilla::TimeDuration timeout = mozilla::TimeDuration::Forever());
 
   template <typename T>
   static typename mozilla::EnableIf<!mozilla::IsBaseOf<Event, T>::value,
                                     void>::Type
   SyncRunEvent(T&& lambda) {
     SyncRunEvent(LambdaEvent<T>(std::forward<T>(lambda)));
   }