Bug 1145201: Implement JS::AutoDebuggerJobQueueInterruption. r=arai,smaug
authorJim Blandy <jimb@mozilla.com>
Tue, 12 Feb 2019 08:14:34 +0000
changeset 458933 b6216c391d41
parent 458932 414b2c238839
child 458934 3ae9f2b94f97
push id35551
push usershindli@mozilla.com
push dateWed, 13 Feb 2019 21:34:09 +0000
treeherdermozilla-central@08f794a4928e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersarai, smaug
bugs1145201
milestone67.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 1145201: Implement JS::AutoDebuggerJobQueueInterruption. r=arai,smaug Define a new RAII class, AutoDebuggerJobQueueInterruption, to save and restore the current ECMAScript job queue, to protect the debuggee's job queue from activity that occurs in debugger callbacks. Add a new method to the JS::JobQueue abstract base class, saveJobQueue, to support AutoDebuggerJobQueueInterruption. Comments on AutoDebuggerJobQueueInterruption provide details. Implement saveJobQueue for SpiderMonkey's internal job queue and for Gecko's job queue in CycleCollectedJSContext. Differential Revision: https://phabricator.services.mozilla.com/D17546
js/public/Promise.h
js/src/builtin/Promise.cpp
js/src/vm/JSContext.cpp
js/src/vm/JSContext.h
xpcom/base/CycleCollectedJSContext.cpp
xpcom/base/CycleCollectedJSContext.h
--- a/js/public/Promise.h
+++ b/js/public/Promise.h
@@ -2,22 +2,28 @@
  * vim: set ts=8 sts=4 et sw=4 tw=99:
  * 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/. */
 
 #ifndef js_Promise_h
 #define js_Promise_h
 
+#include "mozilla/Attributes.h"
+#include "mozilla/GuardObjects.h"
+
 #include "jspubtd.h"
 #include "js/RootingAPI.h"
 #include "js/TypeDecls.h"
+#include "js/UniquePtr.h"
 
 namespace JS {
 
+class JS_PUBLIC_API AutoDebuggerJobQueueInterruption;
+
 /**
  * Abstract base class for an ECMAScript Job Queue:
  * https://www.ecma-international.org/ecma-262/9.0/index.html#sec-jobs-and-job-queues
  *
  * SpiderMonkey doesn't schedule Promise resolution jobs itself; instead, the
  * embedding can provide an instance of this class SpiderMonkey can use to do
  * that scheduling.
  *
@@ -52,33 +58,210 @@ class JS_PUBLIC_API JobQueue {
    * Run all jobs in the queue. Running one job may enqueue others; continue to
    * run jobs until the queue is empty.
    *
    * Calling this method at the wrong time can break the web. The HTML spec
    * indicates exactly when the job queue should be drained (in HTML jargon,
    * when it should "perform a microtask checkpoint"), and doing so at other
    * times can incompatibly change the semantics of programs that use promises
    * or other microtask-based features.
+   *
+   * This method is called only via AutoDebuggerJobQueueInterruption, used by
+   * the Debugger API implementation to ensure that the debuggee's job queue is
+   * protected from the debugger's own activity. See the comments on
+   * AutoDebuggerJobQueueInterruption.
    */
   virtual void runJobs(JSContext* cx) = 0;
 
   /**
    * Return true if the job queue is empty, false otherwise.
    */
   virtual bool empty() const = 0;
+
+ protected:
+  friend class AutoDebuggerJobQueueInterruption;
+
+  /**
+   * A saved job queue, represented however the JobQueue implementation pleases.
+   * Use AutoDebuggerJobQueueInterruption rather than trying to construct one of
+   * these directly; see documentation there.
+   *
+   * Destructing an instance of this class should assert that the current queue
+   * is empty, and then restore the queue the instance captured.
+   */
+  class SavedJobQueue {
+   public:
+    virtual ~SavedJobQueue() = default;
+  };
+
+  /**
+   * Capture this JobQueue's current job queue as a SavedJobQueue and return it,
+   * leaving the JobQueue's job queue empty. Destroying the returned object
+   * should assert that this JobQueue's current job queue is empty, and restore
+   * the original queue.
+   *
+   * On OOM, this should call JS_ReportOutOfMemory on the given JSContext,
+   * and return a null UniquePtr.
+   */
+  virtual js::UniquePtr<SavedJobQueue> saveJobQueue(JSContext*) = 0;
 };
 
 /**
  * Tell SpiderMonkey to use `queue` to schedule promise reactions.
  *
  * SpiderMonkey does not take ownership of the queue; it is the embedding's
  * responsibility to clean it up after the runtime is destroyed.
  */
 extern JS_PUBLIC_API void SetJobQueue(JSContext* cx, JobQueue* queue);
 
+/**
+ * [SMDOC] Protecting the debuggee's job/microtask queue from debugger activity.
+ *
+ * When the JavaScript debugger interrupts the execution of some debuggee code
+ * (for a breakpoint, for example), the debuggee's execution must be paused
+ * while the developer takes time to look at it. During this interruption, other
+ * tabs should remain active and usable. If the debuggee shares a main thread
+ * with non-debuggee tabs, that means that the thread will have to process
+ * non-debuggee HTML tasks and microtasks as usual, even as the debuggee's are
+ * on hold until the debugger lets it continue execution. (Letting debuggee
+ * microtasks run during the interruption would mean that, from the debuggee's
+ * point of view, their side effects would take place wherever the breakpoint
+ * was set - in general, not a place other code should ever run, and a violation
+ * of the run-to-completion rule.)
+ *
+ * This means that, even though the timing and ordering of microtasks is
+ * carefully specified by the standard - and important to preserve for
+ * compatibility and predictability - debugger use may, correctly, have the
+ * effect of reordering microtasks. During the interruption, microtasks enqueued
+ * by non-debuggee tabs must run immediately alongside their HTML tasks as
+ * usual, whereas any debuggee microtasks that were in the queue when the
+ * interruption began must wait for the debuggee to be continued - and thus run
+ * after microtasks enqueued after they were.
+ *
+ * Fortunately, this reordering is visible olny at the global level: when
+ * implemented correctly, it is not detectable by an individual debuggee. Note
+ * that a debuggee should generally be a complete unit of similar-origin related
+ * browsing contexts. Since non-debuggee activity falls outside that unit, it
+ * should never be visible to the debuggee (except via mechanisms that are
+ * already asynchronous, like events), so the debuggee should be unable to
+ * detect non-debuggee microtasks running when they normally would not. As long
+ * as behavior *visible to the debuggee* is unaffected by the interruption, we
+ * have respected the spirit of the rule.
+ *
+ * Of course, even as we accept the general principle that interrupting the
+ * debuggee should have as little detectable effect as possible, we still permit
+ * the developer to do things like evaluate expressions at the console that have
+ * arbitrary effects on the debuggee's state—effects that could never occur
+ * naturally at that point in the program. But since these are explicitly
+ * requested by the developer, who presumably knows what they're doing, we
+ * support this as best we can. If the developer evaluates an expression in the
+ * console that resolves a promise, it seems most natural for the promise's
+ * reaction microtasks to run immediately, within the interruption. This is an
+ * 'unnatural' time for the microtasks to run, but no more unnatural than the
+ * evaluation that triggered them.
+ *
+ * So the overall behavior we need is as follows:
+ *
+ * - When the debugger interrupts a debuggee, the debuggee's microtask queue
+ *   must be saved.
+ *
+ * - When debuggee execution resumes, the debuggee's microtask queue must be
+ *   restored exactly as it was when the interruption occurred.
+ *
+ * - Non-debuggee task and microtask execution must take place normally during
+ *   the interruption.
+ *
+ * Since each HTML task begins with an empty microtask queue, and it should not
+ * be possible for a task to mix debuggee and non-debuggee code, interrupting a
+ * debuggee should always find a microtask queue containing exclusively debuggee
+ * microtasks, if any. So saving and restoring the microtask queue should affect
+ * only the debuggee, not any non-debuggee content.
+ *
+ * AutoDebuggerJobQueueInterruption
+ * --------------------------------
+ *
+ * AutoDebuggerJobQueueInterruption is an RAII class, meant for use by the
+ * Debugger API implementation, that takes care of saving and restoring the
+ * queue.
+ *
+ * Constructing and initializing an instance of AutoDebuggerJobQueueInterruption
+ * sets aside the given JSContext's job queue, leaving the JSContext's queue
+ * empty. When the AutoDebuggerJobQueueInterruption instance is destroyed, it
+ * asserts that the JSContext's current job queue (holding jobs enqueued while
+ * the AutoDebuggerJobQueueInterruption was alive) is empty, and restores the
+ * saved queue to the JSContext.
+ *
+ * Since the Debugger API's behavior is up to us, we can specify that Debugger
+ * hooks begin execution with an empty job queue, and that we drain the queue
+ * after each hook function has run. This drain will be visible to debugger
+ * hooks, and makes hook calls resemble HTML tasks, with their own automatic
+ * microtask checkpoint. But, the drain will be invisible to the debuggee, as
+ * its queue is preserved across the hook invocation.
+ *
+ * To protect the debuggee's job queue, Debugger takes care to invoke callback
+ * functions only within the scope of an AutoDebuggerJobQueueInterruption
+ * instance.
+ *
+ * Why not let the hook functions themselves take care of this?
+ * ------------------------------------------------------------
+ *
+ * Certainly, we could leave responsibility for saving and restoring the job
+ * queue to the Debugger hook functions themselves.
+ *
+ * In fact, early versions of this change tried making the devtools server save
+ * and restore the queue explicitly, but because hooks are set and changed in
+ * numerous places, it was hard to be confident that every case had been
+ * covered, and it seemed that future changes could easily introduce new holes.
+ *
+ * Later versions of this change modified the accessor properties on the
+ * Debugger objects' prototypes to automatically protect the job queue when
+ * calling hooks, but the effect was essentially a monkeypatch applied to an API
+ * we defined and control, which doesn't make sense.
+ *
+ * In the end, since promises have become such a pervasive part of JavaScript
+ * programming, almost any imaginable use of Debugger would need to provide some
+ * kind of protection for the debuggee's job queue, so it makes sense to simply
+ * handle it once, carefully, in the implementation of Debugger itself.
+ */
+class MOZ_RAII JS_PUBLIC_API AutoDebuggerJobQueueInterruption {
+ public:
+  explicit AutoDebuggerJobQueueInterruption(
+      MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM);
+  ~AutoDebuggerJobQueueInterruption();
+
+  bool init(JSContext* cx);
+  bool initialized() const { return !!saved; }
+
+  /**
+   * Drain the job queue. (In HTML terminology, perform a microtask checkpoint.)
+   *
+   * To make Debugger hook calls more like HTML tasks or ECMAScript jobs,
+   * Debugger promises that each hook begins execution with a clean microtask
+   * queue, and that a microtask checkpoint (queue drain) takes place after each
+   * hook returns, successfully or otherwise.
+   *
+   * To ensure these debugger-introduced microtask checkpoints serve only the
+   * hook's microtasks, and never affect the debuggee's, the Debugger API
+   * implementation uses only this method to perform the checkpoints, thereby
+   * statically ensuring that an AutoDebuggerJobQueueInterruption is in scope to
+   * protect the debuggee.
+   *
+   * SavedJobQueue implementations are required to assert that the queue is
+   * empty before restoring the debuggee's queue. If the Debugger API ever fails
+   * to perform a microtask checkpoint after calling a hook, that assertion will
+   * fail, catching the mistake.
+   */
+  void runJobs();
+
+ private:
+  MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER;
+  JSContext* cx;
+  js::UniquePtr<JobQueue::SavedJobQueue> saved;
+};
+
 enum class PromiseRejectionHandlingState { Unhandled, Handled };
 
 typedef void (*PromiseRejectionTrackerCallback)(
     JSContext* cx, JS::HandleObject promise,
     JS::PromiseRejectionHandlingState state, void* data);
 
 /**
  * Sets the callback that's invoked whenever a Promise is rejected without
--- a/js/src/builtin/Promise.cpp
+++ b/js/src/builtin/Promise.cpp
@@ -6,16 +6,17 @@
 
 #include "builtin/Promise.h"
 
 #include "mozilla/Atomics.h"
 #include "mozilla/Maybe.h"
 #include "mozilla/Move.h"
 #include "mozilla/TimeStamp.h"
 
+#include "jsapi.h"
 #include "jsexn.h"
 #include "jsfriendapi.h"
 
 #include "gc/Heap.h"
 #include "js/Debug.h"
 #include "js/PropertySpec.h"
 #include "vm/AsyncFunction.h"
 #include "vm/AsyncIteration.h"
@@ -5166,16 +5167,38 @@ void OffThreadPromiseRuntimeState::shutd
   numCanceled_ = 0;
 
   // After shutdown, there should be no OffThreadPromiseTask activity in this
   // JSRuntime. Revert to the !initialized() state to catch bugs.
   dispatchToEventLoopCallback_ = nullptr;
   MOZ_ASSERT(!initialized());
 }
 
+JS::AutoDebuggerJobQueueInterruption::AutoDebuggerJobQueueInterruption(
+    MOZ_GUARD_OBJECT_NOTIFIER_ONLY_PARAM_IN_IMPL)
+    : cx(nullptr) {
+  MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+}
+
+JS::AutoDebuggerJobQueueInterruption::~AutoDebuggerJobQueueInterruption() {
+  MOZ_ASSERT_IF(cx, cx->jobQueue->empty());
+}
+
+bool JS::AutoDebuggerJobQueueInterruption::init(JSContext* cx) {
+  MOZ_ASSERT(cx->jobQueue);
+  this->cx = cx;
+  saved = cx->jobQueue->saveJobQueue(cx);
+  return !!saved;
+}
+
+void JS::AutoDebuggerJobQueueInterruption::runJobs() {
+  JS::AutoSaveExceptionState ases(cx);
+  cx->jobQueue->runJobs(cx);
+}
+
 const JSJitInfo promise_then_info = {
     {(JSJitGetterOp)Promise_then_noRetVal},
     {0}, /* unused */
     {0}, /* unused */
     JSJitInfo::IgnoresReturnValueNative,
     JSJitInfo::AliasEverything,
     JSVAL_TYPE_UNDEFINED,
 };
--- a/js/src/vm/JSContext.cpp
+++ b/js/src/vm/JSContext.cpp
@@ -1144,16 +1144,48 @@ bool InternalJobQueue::empty() const { r
 JSObject* InternalJobQueue::maybeFront() const {
   if (queue.empty()) {
     return nullptr;
   }
 
   return queue.get().front();
 }
 
+class js::InternalJobQueue::SavedQueue : public JobQueue::SavedJobQueue {
+ public:
+  SavedQueue(JSContext* cx, Queue&& saved)
+      : cx(cx), saved(cx, std::move(saved)) {
+    MOZ_ASSERT(cx->internalJobQueue.ref());
+  }
+
+  ~SavedQueue() {
+    MOZ_ASSERT(cx->internalJobQueue.ref());
+    cx->internalJobQueue->queue = std::move(saved.get());
+  }
+
+ private:
+  JSContext* cx;
+  PersistentRooted<Queue> saved;
+};
+
+js::UniquePtr<JS::JobQueue::SavedJobQueue> InternalJobQueue::saveJobQueue(
+    JSContext* cx) {
+  auto saved = js::MakeUnique<SavedQueue>(cx, std::move(queue.get()));
+  if (!saved) {
+    // When MakeUnique's allocation fails, the SavedQueue constructor is never
+    // called, so this->queue is still initialized. (The move doesn't occur
+    // until the constructor gets called.)
+    ReportOutOfMemory(cx);
+    return nullptr;
+  }
+
+  queue = Queue(SystemAllocPolicy());
+  return saved;
+}
+
 JS::Error JSContext::reportedError;
 JS::OOM JSContext::reportedOOM;
 
 mozilla::GenericErrorResult<OOM&> JSContext::alreadyReportedOOM() {
 #ifdef DEBUG
   if (helperThread()) {
     // Keep in sync with addPendingOutOfMemory.
     if (ParseTask* task = helperThread()->parseTask()) {
--- a/js/src/vm/JSContext.h
+++ b/js/src/vm/JSContext.h
@@ -101,16 +101,19 @@ class InternalJobQueue : public JS::JobQ
   JS::PersistentRooted<Queue> queue;
 
   // True if we are in the midst of draining jobs from this queue. We use this
   // to avoid re-entry (nested calls simply return immediately).
   bool draining_;
 
   // True if we've been asked to interrupt draining jobs. Set by interrupt().
   bool interrupted_;
+
+  class SavedQueue;
+  js::UniquePtr<JobQueue::SavedJobQueue> saveJobQueue(JSContext*) override;
 };
 
 class AutoLockScriptData;
 
 void ReportOverRecursed(JSContext* cx, unsigned errorNumber);
 
 /* Thread Local Storage slot for storing the context for a thread. */
 extern MOZ_THREAD_LOCAL(JSContext*) TlsContext;
--- a/xpcom/base/CycleCollectedJSContext.cpp
+++ b/xpcom/base/CycleCollectedJSContext.cpp
@@ -274,29 +274,64 @@ bool CycleCollectedJSContext::enqueuePro
   }
   JS::RootedObject jobGlobal(aCx, JS::CurrentGlobalOrNull(aCx));
   RefPtr<PromiseJobRunnable> runnable = new PromiseJobRunnable(
       aPromise, aJob, jobGlobal, aAllocationSite, global);
   DispatchToMicroTask(runnable.forget());
   return true;
 }
 
+// Used only by the SpiderMonkey Debugger API, and even then only via
+// JS::AutoDebuggerJobQueueInterruption, to ensure that the debuggee's queue is
+// not affected; see comments in js/public/Promise.h.
 void CycleCollectedJSContext::runJobs(JSContext* aCx) {
   MOZ_ASSERT(aCx == Context());
   MOZ_ASSERT(Get() == this);
   PerformMicroTaskCheckPoint();
 }
 
 bool CycleCollectedJSContext::empty() const {
   // This is our override of JS::JobQueue::empty. Since that interface is only
   // concerned with the ordinary microtask queue, not the debugger microtask
   // queue, we only report on the former.
   return mPendingMicroTaskRunnables.empty();
 }
 
+// Preserve a debuggee's microtask queue while it is interrupted by the
+// debugger. See the comments for JS::AutoDebuggerJobQueueInterruption.
+class CycleCollectedJSContext::SavedMicroTaskQueue
+    : public JS::JobQueue::SavedJobQueue {
+ public:
+  explicit SavedMicroTaskQueue(CycleCollectedJSContext* ccjs) : ccjs(ccjs) {
+    ccjs->mPendingMicroTaskRunnables.swap(mQueue);
+  }
+
+  ~SavedMicroTaskQueue() {
+    MOZ_RELEASE_ASSERT(ccjs->mPendingMicroTaskRunnables.empty());
+    ccjs->mPendingMicroTaskRunnables.swap(mQueue);
+  }
+
+ private:
+  CycleCollectedJSContext* ccjs;
+  std::queue<RefPtr<MicroTaskRunnable>> mQueue;
+};
+
+js::UniquePtr<JS::JobQueue::SavedJobQueue>
+CycleCollectedJSContext::saveJobQueue(JSContext* cx) {
+  auto saved = js::MakeUnique<SavedMicroTaskQueue>(this);
+  if (!saved) {
+    // When MakeUnique's allocation fails, the SavedMicroTaskQueue constructor
+    // is never called, so mPendingMicroTaskRunnables is still initialized.
+    JS_ReportOutOfMemory(cx);
+    return nullptr;
+  }
+
+  return saved;
+}
+
 /* static */
 void CycleCollectedJSContext::PromiseRejectionTrackerCallback(
     JSContext* aCx, JS::HandleObject aPromise,
     JS::PromiseRejectionHandlingState state, void* aData) {
 #ifdef DEBUG
   CycleCollectedJSContext* self = static_cast<CycleCollectedJSContext*>(aData);
 #endif  // DEBUG
   MOZ_ASSERT(aCx == self->Context());
--- a/xpcom/base/CycleCollectedJSContext.h
+++ b/xpcom/base/CycleCollectedJSContext.h
@@ -230,23 +230,28 @@ class CycleCollectedJSContext
       mConsumedRejections;
   nsTArray<nsCOMPtr<nsISupports /* UncaughtRejectionObserver */>>
       mUncaughtRejectionObservers;
 
   virtual bool IsSystemCaller() const = 0;
 
  private:
   // JS::JobQueue implementation: see js/public/Promise.h.
-  // SpiderMonkey uses this to enqueue promise resolution jobs.
+  // SpiderMonkey uses some of these methods to enqueue promise resolution jobs.
+  // Others protect the debuggee microtask queue from the debugger's
+  // interruptions; see the comments on JS::AutoDebuggerJobQueueInterruption for
+  // details.
   JSObject* getIncumbentGlobal(JSContext* cx) override;
   bool enqueuePromiseJob(JSContext* cx, JS::HandleObject promise,
                          JS::HandleObject job, JS::HandleObject allocationSite,
                          JS::HandleObject incumbentGlobal) override;
   void runJobs(JSContext* cx) override;
   bool empty() const override;
+  class SavedMicroTaskQueue;
+  js::UniquePtr<SavedJobQueue> saveJobQueue(JSContext*) override;
 
  private:
   // A primary context owns the mRuntime. Non-main-thread contexts should always
   // be primary. On the main thread, the primary context should be the first one
   // created and the last one destroyed. Non-primary contexts are used for
   // cooperatively scheduled threads.
   bool mIsPrimaryContext;