Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit f70d518

Browse filesBrowse 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 32e5c39 commit f70d518
Copy full SHA for f70d518

File tree

Expand file treeCollapse file tree

7 files changed

+30
-47
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

7 files changed

+30
-47
lines changed
Open diff view settings
Collapse file

‎src/api/callback.cc‎

Copy file name to clipboardExpand all lines: src/api/callback.cc
+6-5Lines changed: 6 additions & 5 deletions
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

Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+2-7Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -374,13 +374,8 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
374374
->GetFunction(env->context())
375375
.ToLocalChecked()};
376376

377-
Local<Value> result;
378-
if (!ExecuteBootstrapper(env, main_script_id, &parameters, &arguments)
379-
.ToLocal(&result) ||
380-
!task_queue::RunNextTicksNative(env)) {
381-
return MaybeLocal<Value>();
382-
}
383-
return scope.Escape(result);
377+
return scope.EscapeMaybe(
378+
ExecuteBootstrapper(env, main_script_id, &parameters, &arguments));
384379
}
385380

386381
MaybeLocal<Value> StartMainThreadExecution(Environment* env) {
Collapse file

‎src/node_internals.h‎

Copy file name to clipboardExpand all lines: src/node_internals.h
+8-3Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,12 +205,16 @@ v8::MaybeLocal<v8::Value> InternalMakeCallback(
205205

206206
class InternalCallbackScope {
207207
public:
208-
// Tell the constructor whether its `object` parameter may be empty or not.
209-
enum ResourceExpectation { kRequireResource, kAllowEmptyResource };
208+
enum Flags {
209+
// Tell the constructor whether its `object` parameter may be empty or not.
210+
kAllowEmptyResource = 1,
211+
// Indicates whether 'before' and 'after' hooks should be skipped.
212+
kSkipAsyncHooks = 2
213+
};
210214
InternalCallbackScope(Environment* env,
211215
v8::Local<v8::Object> object,
212216
const async_context& asyncContext,
213-
ResourceExpectation expect = kRequireResource);
217+
int flags = 0);
214218
// Utility that can be used by AsyncWrap classes.
215219
explicit InternalCallbackScope(AsyncWrap* async_wrap);
216220
~InternalCallbackScope();
@@ -224,6 +228,7 @@ class InternalCallbackScope {
224228
async_context async_context_;
225229
v8::Local<v8::Object> object_;
226230
AsyncCallbackScope callback_scope_;
231+
bool skip_hooks_;
227232
bool failed_ = false;
228233
bool pushed_ids_ = false;
229234
bool closed_ = false;
Collapse file

‎src/node_main_instance.cc‎

Copy file name to clipboardExpand all lines: src/node_main_instance.cc
+7-3Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ using v8::HandleScope;
1818
using v8::Isolate;
1919
using v8::Local;
2020
using v8::Locker;
21+
using v8::Object;
2122
using v8::SealHandleScope;
2223

2324
NodeMainInstance::NodeMainInstance(Isolate* isolate,
@@ -112,10 +113,13 @@ int NodeMainInstance::Run() {
112113

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

121125
env->set_trace_sync_io(env->options()->trace_sync_io);
Collapse file

‎src/node_process.h‎

Copy file name to clipboardExpand all lines: src/node_process.h
-9Lines changed: 0 additions & 9 deletions
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_
Collapse file

‎src/node_task_queue.cc‎

Copy file name to clipboardExpand all lines: src/node_task_queue.cc
-16Lines changed: 0 additions & 16 deletions
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
}
Collapse file

‎src/node_worker.cc‎

Copy file name to clipboardExpand all lines: src/node_worker.cc
+7-4Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -254,17 +254,20 @@ void Worker::Run() {
254254
env_->InitializeInspector(std::move(inspector_parent_handle_));
255255
#endif
256256
HandleScope handle_scope(isolate_);
257-
AsyncCallbackScope callback_scope(env_.get());
258-
env_->async_hooks()->push_async_ids(1, 0);
257+
InternalCallbackScope callback_scope(
258+
env_.get(),
259+
Local<Object>(),
260+
{ 1, 0 },
261+
InternalCallbackScope::kAllowEmptyResource |
262+
InternalCallbackScope::kSkipAsyncHooks);
263+
259264
if (!env_->RunBootstrapping().IsEmpty()) {
260265
CreateEnvMessagePort(env_.get());
261266
if (is_stopped()) return;
262267
Debug(this, "Created message port for worker %llu", thread_id_);
263268
USE(StartExecution(env_.get(), "internal/main/worker_thread"));
264269
}
265270

266-
env_->async_hooks()->pop_async_id(1);
267-
268271
Debug(this, "Loaded environment for worker %llu", thread_id_);
269272
}
270273

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.