Bug 1450644 - Better shutdown approach for Workers - part 3 - Preference for time worker timeout, r=asuth
☠☠ backed out by 73615fe67ab6 ☠ ☠
authorAndrea Marchesini <amarchesini@mozilla.com>
Tue, 17 Apr 2018 20:51:04 +0200
changeset 414181 08239799d43e6ddb85c9d149622151702ddac6f6
parent 414180 cbe3ad4833b670227c4b229dbbe29954d4b9fc25
child 414182 f4989e0da2216f6cf3fbe3d3a616d31447f068ec
push id33861
push userccoroiu@mozilla.com
push dateWed, 18 Apr 2018 10:50:38 +0000
treeherdermozilla-central@4af4ae0aee55 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersasuth
bugs1450644
milestone61.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 1450644 - Better shutdown approach for Workers - part 3 - Preference for time worker timeout, r=asuth
dom/base/DOMPrefs.cpp
dom/base/DOMPrefs.h
dom/base/DOMPrefsInternal.h
dom/workers/WorkerPrivate.cpp
dom/workers/test/test_WorkerDebugger.xul
modules/libpref/Preferences.cpp
--- a/dom/base/DOMPrefs.cpp
+++ b/dom/base/DOMPrefs.cpp
@@ -18,21 +18,23 @@ DOMPrefs::Initialize()
 
   // Let's cache all the values on the main-thread.
 #if !(defined(DEBUG) || defined(MOZ_ENABLE_JS_DUMP))
   DOMPrefs::DumpEnabled();
 #endif
 
 #define DOM_PREF(name, pref) DOMPrefs::name();
 #define DOM_WEBIDL_PREF(name)
+#define DOM_UINT32_PREF(name, pref, defaultValue) DOMPrefs::name();
 
 #include "DOMPrefsInternal.h"
 
 #undef DOM_PREF
 #undef DOM_WEBIDL_PREF
+#undef DOM_UINT32_PREF
 }
 
 #define DOM_PREF(name, pref)                                         \
   /* static */ bool                                                  \
   DOMPrefs::name()                                                   \
   {                                                                  \
     static bool initialized = false;                                 \
     static Atomic<bool> cachedValue;                                 \
@@ -45,25 +47,39 @@ DOMPrefs::Initialize()
 
 #define DOM_WEBIDL_PREF(name)                    \
   /* static */ bool                              \
   DOMPrefs::name(JSContext* aCx, JSObject* aObj) \
   {                                              \
     return DOMPrefs::name();                     \
   }
 
+#define DOM_UINT32_PREF(name, pref, defaultValue)                             \
+  /* static */ uint32_t                                                       \
+  DOMPrefs::name()                                                            \
+  {                                                                           \
+      static bool initialized = false;                                        \
+      static Atomic<uint32_t> cachedValue;                                    \
+      if (!initialized) {                                                     \
+        initialized = true;                                                   \
+        Preferences::AddAtomicUintVarCache(&cachedValue, pref, defaultValue); \
+    }                                                                         \
+    return cachedValue;                                                       \
+  }
+
 #if !(defined(DEBUG) || defined(MOZ_ENABLE_JS_DUMP))
 DOM_PREF(DumpEnabled, "browser.dom.window.dump.enabled")
 #else
 /* static */ bool
 DOMPrefs::DumpEnabled()
 {
   return true;
 }
 #endif
 
 #include "DOMPrefsInternal.h"
 
 #undef DOM_PREF
 #undef DOM_WEBIDL_PREF
+#undef DOM_UINT32_PREF
 
 } // dom namespace
 } // mozilla namespace
--- a/dom/base/DOMPrefs.h
+++ b/dom/base/DOMPrefs.h
@@ -16,19 +16,21 @@ public:
   // This must be called on the main-thread.
   static void Initialize();
 
   // Returns true if the browser.dom.window.dump.enabled pref is set.
   static bool DumpEnabled();
 
 #define DOM_PREF(name, pref) static bool name();
 #define DOM_WEBIDL_PREF(name) static bool name(JSContext* aCx, JSObject* aObj);
+#define DOM_UINT32_PREF(name, pref, defaultValue) static uint32_t name();
 
 #include "DOMPrefsInternal.h"
 
 #undef DOM_PREF
 #undef DOM_WEBIDL_PREF
+#undef DOM_UINT32_PREF
 };
 
 } // dom namespace
 } // mozilla namespace
 
 #endif // mozilla_dom_DOMPrefs_h
--- a/dom/base/DOMPrefsInternal.h
+++ b/dom/base/DOMPrefsInternal.h
@@ -51,8 +51,12 @@ DOM_WEBIDL_PREF(StorageManagerEnabled)
 DOM_WEBIDL_PREF(PromiseRejectionEventsEnabled)
 DOM_WEBIDL_PREF(PushEnabled)
 DOM_WEBIDL_PREF(StreamsEnabled)
 DOM_WEBIDL_PREF(OffscreenCanvasEnabled)
 DOM_WEBIDL_PREF(WebkitBlinkDirectoryPickerEnabled)
 DOM_WEBIDL_PREF(NetworkInformationEnabled)
 DOM_WEBIDL_PREF(FetchObserverEnabled)
 DOM_WEBIDL_PREF(PerformanceObserverEnabled)
+
+DOM_UINT32_PREF(WorkerCancelingTimeoutMillis,
+                "dom.worker.canceling.timeoutMilliseconds",
+                30000 /* 30 seconds */)
--- a/dom/workers/WorkerPrivate.cpp
+++ b/dom/workers/WorkerPrivate.cpp
@@ -3,16 +3,17 @@
 /* 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/. */
 
 #include "WorkerPrivate.h"
 
 #include "js/MemoryMetrics.h"
 #include "MessageEventRunnable.h"
+#include "mozilla/ScopeExit.h"
 #include "mozilla/dom/ClientManager.h"
 #include "mozilla/dom/ClientSource.h"
 #include "mozilla/dom/ClientState.h"
 #include "mozilla/dom/Console.h"
 #include "mozilla/dom/DOMTypes.h"
 #include "mozilla/dom/ErrorEvent.h"
 #include "mozilla/dom/ErrorEventBinding.h"
 #include "mozilla/dom/Event.h"
@@ -67,18 +68,16 @@
 #endif
 
 // JS_MaybeGC will run once every second during normal execution.
 #define PERIODIC_GC_TIMER_DELAY_SEC 1
 
 // A shrinking GC will run five seconds after the last event is processed.
 #define IDLE_GC_TIMER_DELAY_SEC 5
 
-#define CANCELING_TIMEOUT 30000 // 30 seconds
-
 static mozilla::LazyLogModule sWorkerPrivateLog("WorkerPrivate");
 static mozilla::LazyLogModule sWorkerTimeoutsLog("WorkerTimeouts");
 
 mozilla::LogModule*
 WorkerLog()
 {
   return sWorkerPrivateLog;
 }
@@ -4964,17 +4963,17 @@ WorkerPrivate::RescheduleTimeoutTimer(JS
   return true;
 }
 
 void
 WorkerPrivate::StartCancelingTimer()
 {
   AssertIsOnParentThread();
 
-  auto raii = MakeScopeExit([&] {
+  auto errorCleanup = MakeScopeExit([&] {
     mCancelingTimer = nullptr;
   });
 
   MOZ_ASSERT(!mCancelingTimer);
 
   if (WorkerPrivate* parent = GetParent()) {
     mCancelingTimer = NS_NewTimer(parent->ControlEventTarget());
   } else {
@@ -4988,24 +4987,27 @@ WorkerPrivate::StartCancelingTimer()
   // This is not needed if we are already in an advanced shutdown state.
   {
     MutexAutoLock lock(mMutex);
     if (ParentStatus() >= Terminating) {
       return;
     }
   }
 
+  uint32_t cancelingTimeoutMillis = DOMPrefs::WorkerCancelingTimeoutMillis();
+
   RefPtr<CancelingTimerCallback> callback = new CancelingTimerCallback(this);
-  nsresult rv = mCancelingTimer->InitWithCallback(callback, CANCELING_TIMEOUT,
+  nsresult rv = mCancelingTimer->InitWithCallback(callback,
+                                                  cancelingTimeoutMillis,
                                                   nsITimer::TYPE_ONE_SHOT);
   if (NS_WARN_IF(NS_FAILED(rv))) {
     return;
   }
 
-  raii.release();
+  errorCleanup.release();
 }
 
 void
 WorkerPrivate::UpdateContextOptionsInternal(
                                     JSContext* aCx,
                                     const JS::ContextOptions& aContextOptions)
 {
   AssertIsOnWorkerThread();
--- a/dom/workers/test/test_WorkerDebugger.xul
+++ b/dom/workers/test/test_WorkerDebugger.xul
@@ -18,16 +18,17 @@
 
     const WORKER_URL = "WorkerDebugger_worker.js";
     const CHILD_WORKER_URL = "WorkerDebugger_childWorker.js";
     const SHARED_WORKER_URL = "WorkerDebugger_sharedWorker.js";
 
     function test() {
       (async function() {
         SimpleTest.waitForExplicitFinish();
+        SimpleTest.requestLongerTimeout(5);
 
         info("Create a top-level chrome worker that creates a non-top-level " +
              "content worker and wait for their debuggers to be registered.");
         let promise = waitForMultiple([
           waitForRegister(WORKER_URL),
           waitForRegister(CHILD_WORKER_URL)
         ]);
         worker = new ChromeWorker(WORKER_URL);
@@ -51,28 +52,28 @@
         is(childDbg.parent, dbg,
            "Non-top-level worker debugger should have parent.");
         is(childDbg.type, Ci.nsIWorkerDebugger.TYPE_DEDICATED,
            "Content worker debugger should be dedicated.");
         is(childDbg.window, null,
            "Non-top-level worker debugger should not have window.");
 
         info("Terminate the top-level chrome worker and the non-top-level " +
-             "content worker, and wait for their debuggers to be " + 
+             "content worker, and wait for their debuggers to be " +
              "unregistered and closed.");
         promise = waitForMultiple([
           waitForUnregister(CHILD_WORKER_URL),
           waitForDebuggerClose(childDbg),
           waitForUnregister(WORKER_URL),
           waitForDebuggerClose(dbg),
         ]);
         worker.terminate();
         await promise;
 
-        info("Create a shared worker and wait for its debugger to be " + 
+        info("Create a shared worker and wait for its debugger to be " +
              "registered");
         promise = waitForRegister(SHARED_WORKER_URL);
         worker = new SharedWorker(SHARED_WORKER_URL);
         let sharedDbg = await promise;
 
         info("Check that the shared worker debugger has the correct " +
              "properties.");
         is(sharedDbg.isChrome, false,
@@ -112,16 +113,24 @@
 
         info("Send a message to the shared worker to tell it to close " +
              "itself, then loop forever, and wait for its debugger to be closed.");
         promise = waitForMultiple([
           waitForUnregister(SHARED_WORKER_URL),
           waitForDebuggerClose(sharedDbg)
         ]);
 
+        // When the closing process begins, we schedule a timer to terminate
+        // the worker in case it's in an infinite loop, which is exactly what
+        // we do in this test.  We want a duration long enough that we can be
+        // confident that the infinite loop was entered as measured by
+        // performance.now() and that we terminated it, but not as long as our
+        // 30 second default we currently ship.
+        await SpecialPowers.pushPrefEnv({"set": [[ "dom.worker.canceling.timeoutMilliseconds", 15000 ]]});
+
         worker.port.start();
         worker.port.postMessage("close_loop");
         await promise;
 
         wdm.removeListener(listener);
         SimpleTest.finish();
       })();
     }
--- a/modules/libpref/Preferences.cpp
+++ b/modules/libpref/Preferences.cpp
@@ -4953,16 +4953,22 @@ Preferences::AddAtomicUintVarCache(Atomi
                                    bool);
 
 template nsresult
 Preferences::AddAtomicUintVarCache(Atomic<uint32_t, ReleaseAcquire>*,
                                    const char*,
                                    uint32_t,
                                    bool);
 
+template nsresult
+Preferences::AddAtomicUintVarCache(Atomic<uint32_t, SequentiallyConsistent>*,
+                                   const char*,
+                                   uint32_t,
+                                   bool);
+
 static void
 FloatVarChanged(const char* aPref, void* aClosure)
 {
   CacheData* cache = static_cast<CacheData*>(aClosure);
   *static_cast<float*>(cache->mCacheLocation) =
     Preferences::GetFloat(aPref, cache->mDefaultValueFloat);
 }