Commit 4f3d7bb
src: introduce convenience node::MakeSyncCallback()
There are situations where one wants to invoke a JS callback's ->Call()
from C++ and in particular retain any existing async_context state, but
where it's not obvious that a plain ->Call() would be safe at the point
in question.
Such callsites usually resort to
node::MakeCallback(..., async_context{0, 0}), which unconditionally
pushes the async_context{0, 0} and takes the required provisions for the
->Call() itself such as triggering the tick after its return, if needed.
An example would be the PerformanceObserver invocation from
PerformanceEntry::Notify(): this can get called when coming from JS
through e.g. perf_hooks.performance.mark() and alike, but perhaps also
from nghttp2 (c.f. EmitStatistics() in node_http2.cc).
In the former case, a plain ->Call() would be safe and it would be
desirable to retain the current async_context so that
PerformanceObservers can access it resp. the associated
AsyncLocalStorage. However, in the second case the additional provisions
taken by node::MakeCallback() might potentially be strictly required.
So PerformanceEntry::Notify() bites the bullet and invokes the
PerformanceObservers through node::MakeCallback() unconditionally,
thereby always rendering any possibly preexisting async_context
inaccessible.
Introduce the convenience node::MakeSyncCallback() for such usecases,
which would basically forward to ->Call() if safe and to
node::MakeCallback(..., async_context{0, 0}) otherwise.
Co-Authored-By: ZauberNerd <zaubernerd@zaubernerd.de>
PR-URL: #36343
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>1 parent e597882 commit 4f3d7bb
2 files changed
Lines changed: 34 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -266,6 +266,34 @@ MaybeLocal<Value> MakeCallback(Isolate* isolate, | |||
| 266 | 266 | return ret; | |
| 267 | 267 | } | |
| 268 | 268 | ||
| 269 | + // Use this if you just want to safely invoke some JS callback and | ||
| 270 | + // would like to retain the currently active async_context, if any. | ||
| 271 | + // In case none is available, a fixed default context will be | ||
| 272 | + // installed otherwise. | ||
| 273 | + MaybeLocal<Value> MakeSyncCallback(Isolate* isolate, | ||
| 274 | + Local<Object> recv, | ||
| 275 | + Local<Function> callback, | ||
| 276 | + int argc, | ||
| 277 | + Local<Value> argv[]) { | ||
| 278 | + Environment* env = Environment::GetCurrent(callback->CreationContext()); | ||
| 279 | + CHECK_NOT_NULL(env); | ||
| 280 | + if (!env->can_call_into_js()) return Local<Value>(); | ||
| 281 | + | ||
| 282 | + Context::Scope context_scope(env->context()); | ||
| 283 | + if (env->async_callback_scope_depth()) { | ||
| 284 | + // There's another MakeCallback() on the stack, piggy back on it. | ||
| 285 | + // In particular, retain the current async_context. | ||
| 286 | + return callback->Call(env->context(), recv, argc, argv); | ||
| 287 | + } | ||
| 288 | + | ||
| 289 | + // This is a toplevel invocation and the caller (intentionally) | ||
| 290 | + // didn't provide any async_context to run in. Install a default context. | ||
| 291 | + MaybeLocal<Value> ret = | ||
| 292 | + InternalMakeCallback(env, env->process_object(), recv, callback, argc, argv, | ||
| 293 | + async_context{0, 0}); | ||
| 294 | + return ret; | ||
| 295 | + } | ||
| 296 | + | ||
| 269 | 297 | // Legacy MakeCallback()s | |
| 270 | 298 | ||
| 271 | 299 | Local<Value> MakeCallback(Isolate* isolate, | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -200,6 +200,12 @@ v8::MaybeLocal<v8::Value> InternalMakeCallback( | |||
| 200 | 200 | v8::Local<v8::Value> argv[], | |
| 201 | 201 | async_context asyncContext); | |
| 202 | 202 | ||
| 203 | + v8::MaybeLocal<v8::Value> MakeSyncCallback(v8::Isolate* isolate, | ||
| 204 | + v8::Local<v8::Object> recv, | ||
| 205 | + v8::Local<v8::Function> callback, | ||
| 206 | + int argc, | ||
| 207 | + v8::Local<v8::Value> argv[]); | ||
| 208 | + | ||
| 203 | 209 | class InternalCallbackScope { | |
| 204 | 210 | public: | |
| 205 | 211 | enum Flags { | |
0 commit comments