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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
James Willcox 2018-12-19 22:59:46 +00:00
parent dc32ae75b8
commit e2e452c637
3 changed files with 36 additions and 16 deletions

View File

@ -5,16 +5,13 @@
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;
@ -134,6 +131,8 @@ public class GeckoThread extends Thread {
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";
@ -576,8 +575,21 @@ public class GeckoThread extends Thread {
"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();

View File

@ -144,7 +144,7 @@ class GeckoThreadSupport final
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; }
@ -160,7 +160,8 @@ class GeckoThreadSupport final
NS_DISPATCH_SYNC);
}
};
nsAppShell::SyncRunEvent(NoOpEvent());
return nsAppShell::SyncRunEvent(
NoOpEvent(), nullptr, TimeDuration::FromMilliseconds(timeoutMillis));
}
static void OnPause() {
@ -686,8 +687,9 @@ bool nsAppShell::ProcessNextNativeEvent(bool mayWait) {
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());
@ -699,19 +701,20 @@ void nsAppShell::SyncRunEvent(
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 =
@ -725,8 +728,10 @@ void nsAppShell::SyncRunEvent(
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) {

View File

@ -13,6 +13,7 @@
#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"
@ -66,7 +67,7 @@ class nsAppShell : public nsBaseAppShell {
public:
explicit LambdaEvent(T&& l) : lambda(std::move(l)) {}
void Run() override { return lambda(); }
void Run() override { lambda(); }
};
class ProxyEvent : public Event {
@ -123,9 +124,11 @@ class nsAppShell : public nsBaseAppShell {
}
// 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,