Skip to content

Commit e3371f0

Browse files
addaleaxMylesBorins
authored andcommitted
src: use callback scope for main script
This allows removing custom code for setting the current async ids and running nextTicks. PR-URL: #30236 Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 8fe6849 commit e3371f0

7 files changed

+30
-47
lines changed

src/api/callback.cc

+6-5
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,13 @@ InternalCallbackScope::InternalCallbackScope(AsyncWrap* async_wrap)
4343
InternalCallbackScope::InternalCallbackScope(Environment* env,
4444
Local<Object> object,
4545
const async_context& asyncContext,
46-
ResourceExpectation expect)
46+
int flags)
4747
: env_(env),
4848
async_context_(asyncContext),
4949
object_(object),
50-
callback_scope_(env) {
51-
CHECK_IMPLIES(expect == kRequireResource, !object.IsEmpty());
50+
callback_scope_(env),
51+
skip_hooks_(flags & kSkipAsyncHooks) {
52+
CHECK_IMPLIES(!(flags & kAllowEmptyResource), !object.IsEmpty());
5253
CHECK_NOT_NULL(env);
5354

5455
if (!env->can_call_into_js()) {
@@ -60,7 +61,7 @@ InternalCallbackScope::InternalCallbackScope(Environment* env,
6061
// If you hit this assertion, you forgot to enter the v8::Context first.
6162
CHECK_EQ(Environment::GetCurrent(env->isolate()), env);
6263

63-
if (asyncContext.async_id != 0) {
64+
if (asyncContext.async_id != 0 && !skip_hooks_) {
6465
// No need to check a return value because the application will exit if
6566
// an exception occurs.
6667
AsyncWrap::EmitBefore(env, asyncContext.async_id);
@@ -89,7 +90,7 @@ void InternalCallbackScope::Close() {
8990

9091
if (failed_) return;
9192

92-
if (async_context_.async_id != 0) {
93+
if (async_context_.async_id != 0 && !skip_hooks_) {
9394
AsyncWrap::EmitAfter(env_, async_context_.async_id);
9495
}
9596

src/node.cc

+2-7
Original file line numberDiff line numberDiff line change
@@ -406,13 +406,8 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
406406
->GetFunction(env->context())
407407
.ToLocalChecked()};
408408

409-
Local<Value> result;
410-
if (!ExecuteBootstrapper(env, main_script_id, &parameters, &arguments)
411-
.ToLocal(&result) ||
412-
!task_queue::RunNextTicksNative(env)) {
413-
return MaybeLocal<Value>();
414-
}
415-
return scope.Escape(result);
409+
return scope.EscapeMaybe(
410+
ExecuteBootstrapper(env, main_script_id, &parameters, &arguments));
416411
}
417412

418413
MaybeLocal<Value> StartMainThreadExecution(Environment* env) {

src/node_internals.h

+8-3
Original file line numberDiff line numberDiff line change
@@ -203,12 +203,16 @@ v8::MaybeLocal<v8::Value> InternalMakeCallback(
203203

204204
class InternalCallbackScope {
205205
public:
206-
// Tell the constructor whether its `object` parameter may be empty or not.
207-
enum ResourceExpectation { kRequireResource, kAllowEmptyResource };
206+
enum Flags {
207+
// Tell the constructor whether its `object` parameter may be empty or not.
208+
kAllowEmptyResource = 1,
209+
// Indicates whether 'before' and 'after' hooks should be skipped.
210+
kSkipAsyncHooks = 2
211+
};
208212
InternalCallbackScope(Environment* env,
209213
v8::Local<v8::Object> object,
210214
const async_context& asyncContext,
211-
ResourceExpectation expect = kRequireResource);
215+
int flags = 0);
212216
// Utility that can be used by AsyncWrap classes.
213217
explicit InternalCallbackScope(AsyncWrap* async_wrap);
214218
~InternalCallbackScope();
@@ -222,6 +226,7 @@ class InternalCallbackScope {
222226
async_context async_context_;
223227
v8::Local<v8::Object> object_;
224228
AsyncCallbackScope callback_scope_;
229+
bool skip_hooks_;
225230
bool failed_ = false;
226231
bool pushed_ids_ = false;
227232
bool closed_ = false;

src/node_main_instance.cc

+7-3
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ using v8::HandleScope;
1414
using v8::Isolate;
1515
using v8::Local;
1616
using v8::Locker;
17+
using v8::Object;
1718
using v8::SealHandleScope;
1819

1920
NodeMainInstance::NodeMainInstance(Isolate* isolate,
@@ -111,10 +112,13 @@ int NodeMainInstance::Run() {
111112

112113
if (exit_code == 0) {
113114
{
114-
AsyncCallbackScope callback_scope(env.get());
115-
env->async_hooks()->push_async_ids(1, 0);
115+
InternalCallbackScope callback_scope(
116+
env.get(),
117+
Local<Object>(),
118+
{ 1, 0 },
119+
InternalCallbackScope::kAllowEmptyResource |
120+
InternalCallbackScope::kSkipAsyncHooks);
116121
LoadEnvironment(env.get());
117-
env->async_hooks()->pop_async_id(1);
118122
}
119123

120124
env->set_trace_sync_io(env->options()->trace_sync_io);

src/node_process.h

-9
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,6 @@ v8::Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
3434
v8::MaybeLocal<v8::Object> CreateProcessObject(Environment* env);
3535
void PatchProcessObject(const v8::FunctionCallbackInfo<v8::Value>& args);
3636

37-
namespace task_queue {
38-
// Handle any nextTicks added in the first tick of the program.
39-
// We use the native version here for once so that any microtasks
40-
// created by the main module is then handled from C++, and
41-
// the call stack of the main script does not show up in the async error
42-
// stack trace.
43-
bool RunNextTicksNative(Environment* env);
44-
} // namespace task_queue
45-
4637
} // namespace node
4738
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
4839
#endif // SRC_NODE_PROCESS_H_

src/node_task_queue.cc

-16
Original file line numberDiff line numberDiff line change
@@ -41,22 +41,6 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
4141
isolate->EnqueueMicrotask(args[0].As<Function>());
4242
}
4343

44-
// Should be in sync with runNextTicks in internal/process/task_queues.js
45-
bool RunNextTicksNative(Environment* env) {
46-
OnScopeLeave weakref_cleanup([&]() { env->RunWeakRefCleanup(); });
47-
48-
TickInfo* tick_info = env->tick_info();
49-
if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn())
50-
MicrotasksScope::PerformCheckpoint(env->isolate());
51-
if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn())
52-
return true;
53-
54-
Local<Function> callback = env->tick_callback_function();
55-
CHECK(!callback.IsEmpty());
56-
return !callback->Call(env->context(), env->process_object(), 0, nullptr)
57-
.IsEmpty();
58-
}
59-
6044
static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
6145
MicrotasksScope::PerformCheckpoint(args.GetIsolate());
6246
}

src/node_worker.cc

+7-4
Original file line numberDiff line numberDiff line change
@@ -347,17 +347,20 @@ void Worker::Run() {
347347
inspector_started = true;
348348
#endif
349349
HandleScope handle_scope(isolate_);
350-
AsyncCallbackScope callback_scope(env_.get());
351-
env_->async_hooks()->push_async_ids(1, 0);
350+
InternalCallbackScope callback_scope(
351+
env_.get(),
352+
Local<Object>(),
353+
{ 1, 0 },
354+
InternalCallbackScope::kAllowEmptyResource |
355+
InternalCallbackScope::kSkipAsyncHooks);
356+
352357
if (!env_->RunBootstrapping().IsEmpty()) {
353358
CreateEnvMessagePort(env_.get());
354359
if (is_stopped()) return;
355360
Debug(this, "Created message port for worker %llu", thread_id_);
356361
USE(StartExecution(env_.get(), "internal/main/worker_thread"));
357362
}
358363

359-
env_->async_hooks()->pop_async_id(1);
360-
361364
Debug(this, "Loaded environment for worker %llu", thread_id_);
362365
}
363366

0 commit comments

Comments
 (0)