Bug 1450644 - Better shutdown approach for Workers - part 2 - Timeout + ControlRunnable, r=asuth

dom/workers/WorkerPrivate.cpp

@@ -72,6 +72,8 @@
 // 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");
 
@@ -995,6 +997,50 @@ public:
   }
 };
 
+// A runnable to cancel the worker from the parent process.
+class CancelingWithTimeoutOnParentRunnable final : public WorkerControlRunnable
+{
+public:
+  explicit CancelingWithTimeoutOnParentRunnable(WorkerPrivate* aWorkerPrivate)
+    : WorkerControlRunnable(aWorkerPrivate, ParentThreadUnchangedBusyCount)
+  {}
+
+  bool
+  WorkerRun(JSContext* aCx, WorkerPrivate* aWorkerPrivate) override
+  {
+    aWorkerPrivate->AssertIsOnParentThread();
+    aWorkerPrivate->StartCancelingTimer();
+    return true;
+  }
+};
+
+class CancelingTimerCallback final : public nsITimerCallback
+{
+public:
+  NS_DECL_ISUPPORTS
+
+  explicit CancelingTimerCallback(WorkerPrivate* aWorkerPrivate)
+    : mWorkerPrivate(aWorkerPrivate)
+  {}
+
+  NS_IMETHOD
+  Notify(nsITimer* aTimer) override
+  {
+    mWorkerPrivate->AssertIsOnParentThread();
+    mWorkerPrivate->Cancel();
+    return NS_OK;
+  }
+
+private:
+  ~CancelingTimerCallback() = default;
+
+  // Raw pointer here is OK because the timer is canceled during the shutdown
+  // steps.
+  WorkerPrivate* mWorkerPrivate;
+};
+
+NS_IMPL_ISUPPORTS(CancelingTimerCallback, nsITimerCallback)
+
 // This runnable starts the canceling of a worker after a self.close().
 class CancelingRunnable final : public Runnable
 {
@@ -1780,6 +1826,12 @@ WorkerPrivate::NotifyPrivate(WorkerStatus aStatus)
   // Anything queued will be discarded.
   mQueuedRunnables.Clear();
 
+  // No Canceling timeout is needed.
+  if (mCancelingTimer) {
+    mCancelingTimer->Cancel();
+    mCancelingTimer = nullptr;
+  }
+
   RefPtr<NotifyRunnable> runnable = new NotifyRunnable(this, aStatus);
   return runnable->Dispatch();
 }
@@ -4538,6 +4590,13 @@ WorkerPrivate::NotifyInternal(WorkerStatus aStatus)
       // WorkerControlRunnable because they are immediately executed.
       RefPtr<CancelingRunnable> r = new CancelingRunnable();
       mThread->nsThread::Dispatch(r.forget(), NS_DISPATCH_NORMAL);
+
+      // At the same time, we want to be sure that we interrupt infinite loops.
+      // The following runnable starts a timer that cancel the worker, from the
+      // parent thread, after CANCELING_TIMEOUT millseconds.
+      RefPtr<CancelingWithTimeoutOnParentRunnable> rr =
+        new CancelingWithTimeoutOnParentRunnable(this);
+      rr->Dispatch();
     }
     return true;
   }
@@ -4905,6 +4964,45 @@ WorkerPrivate::RescheduleTimeoutTimer(JSContext* aCx)
   return true;
 }
 
+void
+WorkerPrivate::StartCancelingTimer()
+{
+  AssertIsOnParentThread();
+
+  auto raii = MakeScopeExit([&] {
+    mCancelingTimer = nullptr;
+  });
+
+  MOZ_ASSERT(!mCancelingTimer);
+
+  if (WorkerPrivate* parent = GetParent()) {
+    mCancelingTimer = NS_NewTimer(parent->ControlEventTarget());
+  } else {
+    mCancelingTimer = NS_NewTimer();
+  }
+
+  if (NS_WARN_IF(!mCancelingTimer)) {
+    return;
+  }
+
+  // This is not needed if we are already in an advanced shutdown state.
+  {
+    MutexAutoLock lock(mMutex);
+    if (ParentStatus() >= Terminating) {
+      return;
+    }
+  }
+
+  RefPtr<CancelingTimerCallback> callback = new CancelingTimerCallback(this);
+  nsresult rv = mCancelingTimer->InitWithCallback(callback, CANCELING_TIMEOUT,
+                                                  nsITimer::TYPE_ONE_SHOT);
+  if (NS_WARN_IF(NS_FAILED(rv))) {
+    return;
+  }
+
+  raii.release();
+}
+
 void
 WorkerPrivate::UpdateContextOptionsInternal(
                                     JSContext* aCx,

dom/workers/WorkerPrivate.h

@@ -1207,6 +1207,9 @@ public:
   PrincipalIsValid() const;
 #endif
 
+  void
+  StartCancelingTimer();
+
 private:
   WorkerPrivate(WorkerPrivate* aParent,
                 const nsAString& aScriptURL, bool aIsChromeWorker,
@@ -1415,6 +1418,8 @@ private:
   nsCOMPtr<nsITimer> mTimer;
   nsCOMPtr<nsITimerCallback> mTimerRunnable;
 
+  nsCOMPtr<nsITimer> mCancelingTimer;
+
   nsCOMPtr<nsITimer> mGCTimer;
 
   RefPtr<MemoryReporter> mMemoryReporter;

dom/workers/test/WorkerDebugger_sharedWorker.js

@@ -6,6 +6,12 @@ self.onconnect = function (event) {
     case "close":
       close();
       break;
+
+    case "close_loop":
+      close();
+      // Let's loop forever.
+      while(1) {}
+      break;
     }
   };
 };

dom/workers/test/test_WorkerDebugger.xul

@@ -93,6 +93,7 @@
           },
         };
         wdm.addListener(listener);
+
         worker = new SharedWorker(SHARED_WORKER_URL);
 
         info("Send a message to the shared worker to tell it to close " +
@@ -105,6 +106,21 @@
         worker.port.postMessage("close");
         await promise;
 
+        promise = waitForRegister(SHARED_WORKER_URL);
+        worker = new SharedWorker(SHARED_WORKER_URL);
+        sharedDbg = await promise;
+
+        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)
+        ]);
+
+        worker.port.start();
+        worker.port.postMessage("close_loop");
+        await promise;
+
         wdm.removeListener(listener);
         SimpleTest.finish();
       })();