From 7b8d7ae83a37f1ca333c6d18aca132a5d651e3a1 Mon Sep 17 00:00:00 2001 From: Jim Blandy Date: Thu, 20 Dec 2018 17:27:51 +0000 Subject: [PATCH] Bug 1145201: Document OffThreadPromiseTask. r=luke Differential Revision: https://phabricator.services.mozilla.com/D14927 --HG-- extra : moz-landing-system : lando --- js/public/Promise.h | 30 ++++++++++------ js/src/builtin/Promise.cpp | 38 +++++++++++++------- js/src/builtin/Promise.h | 71 ++++++++++++++++++++++++++++++++++---- js/src/vm/HelperThreads.h | 21 +++++++++-- 4 files changed, 129 insertions(+), 31 deletions(-) diff --git a/js/public/Promise.h b/js/public/Promise.h index 7f769568d330..cdb5a43f30e6 100644 --- a/js/public/Promise.h +++ b/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, diff --git a/js/src/builtin/Promise.cpp b/js/src/builtin/Promise.cpp index 2ede6a86218f..6f5738ea222a 100644 --- a/js/src/builtin/Promise.cpp +++ b/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 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 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); diff --git a/js/src/builtin/Promise.h b/js/src/builtin/Promise.h index dbcc0248399c..f3ad86632523 100644 --- a/js/src/builtin/Promise.h +++ b/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; diff --git a/js/src/vm/HelperThreads.h b/js/src/vm/HelperThreads.h index 40c117bd131a..39830fc3e2e3 100644 --- a/js/src/vm/HelperThreads.h +++ b/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 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 promise) : OffThreadPromiseTask(cx, promise) {}