Bug 1145201: Document OffThreadPromiseTask. r=luke

Differential Revision: https://phabricator.services.mozilla.com/D14927

--HG--
extra : moz-landing-system : lando

js/public/Promise.h

@@ -308,16 +308,26 @@ class JS_PUBLIC_API Dispatchable {
 };
 
 /**
- * DispatchToEventLoopCallback may be called from any thread, being passed the
- * same 'closure' passed to InitDispatchToEventLoop() and Dispatchable from the
- * same JSRuntime. If the embedding returns 'true', the embedding must call
- * Dispatchable::run() on an active JSContext thread for the same JSRuntime on
- * which 'closure' was registered. If DispatchToEventLoopCallback returns
- * 'false', SpiderMonkey will assume a shutdown of the JSRuntime is in progress.
- * This contract implies that, by the time the final JSContext is destroyed in
- * the JSRuntime, the embedding must have (1) run all Dispatchables for which
- * DispatchToEventLoopCallback returned true, (2) already started returning
- * false from calls to DispatchToEventLoopCallback.
+ * Callback to dispatch a JS::Dispatchable to a JSContext's thread's event loop.
+ *
+ * The DispatchToEventLoopCallback set on a particular JSContext must accept
+ * JS::Dispatchable instances and arrange for their `run` methods to be called
+ * eventually on the JSContext's thread. This is used for cross-thread dispatch,
+ * so the callback itself must be safe to call from any thread.
+ *
+ * If the callback returns `true`, it must eventually run the given
+ * Dispatchable; otherwise, SpiderMonkey may leak memory or hang.
+ *
+ * The callback may return `false` to indicate that the JSContext's thread is
+ * shutting down and is no longer accepting runnables. Shutting down is a
+ * one-way transition: once the callback has rejected a runnable, it must reject
+ * all subsequently submitted runnables as well.
+ *
+ * To establish a DispatchToEventLoopCallback, the embedding may either call
+ * InitDispatchToEventLoop to provide its own, or call js::UseInternalJobQueues
+ * to select a default implementation built into SpiderMonkey. This latter
+ * depends on the embedding to call js::RunJobs on the JavaScript thread to
+ * process queued Dispatchables at appropriate times.
  */
 
 typedef bool (*DispatchToEventLoopCallback)(void* closure,

js/src/builtin/Promise.cpp

@@ -4969,12 +4969,11 @@ void OffThreadPromiseTask::dispatchResolveAndDestroy() {
     return;
   }
 
-  // We assume, by interface contract, that if the dispatch fails, it's
-  // because the embedding is in the process of shutting down the JSRuntime.
-  // Since JSRuntime destruction calls shutdown(), we can rely on shutdown()
-  // to delete the task on its active JSContext thread. shutdown() waits for
-  // numCanceled_ == live_.length, so we notify when this condition is
-  // reached.
+  // The DispatchToEventLoopCallback has rejected this task, indicating that
+  // shutdown has begun. Count the number of rejected tasks that have called
+  // dispatchResolveAndDestroy, and when they account for the entire contents of
+  // live_, notify OffThreadPromiseRuntimeState::shutdown that it is safe to
+  // destruct them.
   LockGuard<Mutex> lock(state.mutex_);
   state.numCanceled_++;
   if (state.numCanceled_ == state.live_.count()) {
@@ -5105,8 +5104,22 @@ void OffThreadPromiseRuntimeState::shutdown(JSContext* cx) {
   }
 
   {
-    // Wait until all live OffThreadPromiseRuntimeState have been confirmed
-    // canceled by OffThreadPromiseTask::dispatchResolve().
+    // An OffThreadPromiseTask may only be safely deleted on its JSContext's
+    // thread (since it contains a PersistentRooted holding its promise), and
+    // only after it has called dispatchResolveAndDestroy (since that is our
+    // only indication that its owner is done writing into it).
+    //
+    // OffThreadPromiseTasks accepted by the DispatchToEventLoopCallback are
+    // deleted by their 'run' methods. Only dispatchResolveAndDestroy invokes
+    // the callback, and the point of the callback is to call 'run' on the
+    // JSContext's thread, so the conditions above are met.
+    //
+    // But although the embedding's DispatchToEventLoopCallback promises to run
+    // every task it accepts before shutdown, when shutdown does begin it starts
+    // rejecting tasks; we cannot count on 'run' to clean those up for us.
+    // Instead, dispatchResolveAndDestroy keeps a count of rejected ('canceled')
+    // tasks; once that count covers everything in live_, this function itself
+    // runs only on the JSContext's thread, so we can delete them all here.
     LockGuard<Mutex> lock(mutex_);
     while (live_.count() != numCanceled_) {
       MOZ_ASSERT(numCanceled_ < live_.count());
@@ -5114,13 +5127,14 @@ void OffThreadPromiseRuntimeState::shutdown(JSContext* cx) {
     }
   }
 
-  // Now that all the tasks have stopped concurrent execution, we can just
-  // delete everything. We don't want each OffThreadPromiseTask to unregister
-  // itself (which would mutate live_ while we are iterating over it) so reset
-  // the tasks' internal registered_ flag.
+  // Now that live_ contains only cancelled tasks, we can just delete
+  // everything.
   for (OffThreadPromiseTaskSet::Range r = live_.all(); !r.empty();
        r.popFront()) {
     OffThreadPromiseTask* task = r.front();
+
+    // We don't want 'task' to unregister itself (which would mutate live_ while
+    // we are iterating over it) so reset its internal registered_ flag.
     MOZ_ASSERT(task->registered_);
     task->registered_ = false;
     js_delete(task);

js/src/builtin/Promise.h

@@ -419,13 +419,70 @@ class MOZ_NON_TEMPORARY_CLASS PromiseLookup final {
   }
 };
 
-// An OffThreadPromiseTask holds a rooted Promise JSObject while executing an
-// off-thread task (defined by the subclass) that needs to resolve the Promise
-// on completion. Because OffThreadPromiseTask contains a PersistentRooted, it
-// must be destroyed on an active JSContext thread of the Promise's JSRuntime.
-// OffThreadPromiseTasks may be run off-thread in various ways (e.g., see
-// PromiseHelperTask). At any time, the task can be dispatched to an active
-// JSContext of the Promise's JSRuntime by calling dispatchResolve().
+// [SMDOC] OffThreadPromiseTask: an off-main-thread task that resolves a promise
+//
+// An OffThreadPromiseTask is an abstract base class holding a JavaScript
+// promise that will be resolved (fulfilled or rejected) with the results of a
+// task possibly performed by some other thread.
+//
+// An OffThreadPromiseTask's lifecycle is as follows:
+//
+// - Some JavaScript native wishes to return a promise of the result of some
+//   computation that might be performed by other threads (say, helper threads
+//   or the embedding's I/O threads), so it creates a PromiseObject to represent
+//   the result, and an OffThreadPromiseTask referring to it. After handing the
+//   OffThreadPromiseTask to the code doing the actual work, the native is free
+//   to return the PromiseObject to its caller.
+//
+// - When the computation is done, successfully or otherwise, it populates the
+//   OffThreadPromiseTask—which is actually an instance of some concrete
+//   subclass specific to the task—with the information needed to resolve the
+//   promise, and calls OffThreadPromiseTask::dispatchResolveAndDestroy. This
+//   enqueues a runnable on the JavaScript thread to which the promise belongs.
+//
+// - When it gets around to the runnable, the JavaScript thread calls the
+//   OffThreadPromiseTask's `resolve` method, which the concrete subclass has
+//   overriden to resolve the promise appropriately. This probably enqueues a
+//   promise reaction job.
+//
+// - The JavaScript thread then deletes the OffThreadPromiseTask.
+//
+// During shutdown, the process is slightly different. Enqueuing runnables to
+// the JavaScript thread begins to fail. JSRuntime shutdown waits for all
+// outstanding tasks to call dispatchResolveAndDestroy, and then deletes them on
+// the main thread, without calling `resolve`.
+//
+// For example, the JavaScript function WebAssembly.compile uses
+// OffThreadPromiseTask to manage the result of a helper thread task, accepting
+// binary WebAssembly code and returning a promise of a compiled
+// WebAssembly.Module. It would like to do this compilation work on a helper
+// thread. When called by JavaScript, WebAssembly.compile creates a promise,
+// builds a CompileBufferTask (the OffThreadPromiseTask concrete subclass) to
+// keep track of it, and then hands that to a helper thread. When the helper
+// thread is done, successfully or otherwise, it calls the CompileBufferTask's
+// dispatchResolveAndDestroy method, which enqueues a runnable to the JavaScript
+// thread to resolve the promise and delete the CompileBufferTask.
+// (CompileBufferTask actually implements PromiseHelperTask, which implements
+// OffThreadPromiseTask; PromiseHelperTask is what our helper thread scheduler
+// requires.)
+//
+// OffThreadPromiseTasks are not limited to use with helper threads. For
+// example, a function returning a promise of the result of a network operation
+// could provide the code collecting the incoming data with an
+// OffThreadPromiseTask for the promise, and let the embedding's network I/O
+// threads call dispatchResolveAndDestroy.
+//
+// An OffThreadPromiseTask has a JSContext, and must be constructed and have its
+// 'init' method called on that JSContext's thread. Once initialized, its
+// dispatchResolveAndDestroy method may be called from any thread. This is the
+// only safe way to destruct an OffThreadPromiseTask; doing so ensures the
+// OffThreadPromiseTask's destructor will run on the JSContext's thread, either
+// from the event loop or during shutdown.
+//
+// OffThreadPromiseTask::dispatchResolveAndDestroy uses the
+// JS::DispatchToEventLoopCallback provided by the embedding to enqueue
+// runnables on the JavaScript thread. See the comments for
+// DispatchToEventLoopCallback for details.
 
 class OffThreadPromiseTask : public JS::Dispatchable {
   friend class OffThreadPromiseRuntimeState;

js/src/vm/HelperThreads.h

@@ -519,10 +519,22 @@ void CancelOffThreadWasmTier2Generator();
  * If helper threads are available, call execute() then dispatchResolve() on the
  * given task in a helper thread. If no helper threads are available, the given
  * task is executed and resolved synchronously.
+ *
+ * This function takes ownership of task unconditionally; if it fails, task is
+ * deleted.
  */
 bool StartOffThreadPromiseHelperTask(JSContext* cx,
                                      UniquePtr<PromiseHelperTask> task);
 
+/*
+ * Like the JSContext-accepting version, but only safe to use when helper
+ * threads are available, so we can be sure we'll never need to fall back on
+ * synchronous execution.
+ *
+ * This function can be called from any thread, but takes ownership of the task
+ * only on success. On OOM, it is the caller's responsibility to arrange for the
+ * task to be cleaned up properly.
+ */
 bool StartOffThreadPromiseHelperTask(PromiseHelperTask* task);
 
 /*
@@ -838,8 +850,13 @@ class SourceCompressionTask {
 };
 
 // A PromiseHelperTask is an OffThreadPromiseTask that executes a single job on
-// a helper thread. Derived classes do their helper-thread work by implementing
-// execute().
+// a helper thread. Call js::StartOffThreadPromiseHelperTask to submit a
+// PromiseHelperTask for execution.
+//
+// Concrete subclasses must implement execute and OffThreadPromiseTask::resolve.
+// The helper thread will call execute() to do the main work. Then, the thread
+// of the JSContext used to create the PromiseHelperTask will call resolve() to
+// resolve promise according to those results.
 struct PromiseHelperTask : OffThreadPromiseTask {
   PromiseHelperTask(JSContext* cx, Handle<PromiseObject*> promise)
       : OffThreadPromiseTask(cx, promise) {}