FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: use callback scope for main script · nodejs/node@ce13d43 · GitHub

/ node Public

Commit ce13d43

Browse files
addaleax
authored and
BethGriggs
committed
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 e11a376 commit ce13d43

7 files changed

Lines changed: 30 additions & 47 deletions

File tree

‎src/api/callback.cc‎

Lines 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

‎src/node.cc‎

Lines 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) {

‎src/node_internals.h‎

Lines 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;

‎src/node_main_instance.cc‎

Lines 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);

‎src/node_process.h‎

Lines 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_

‎src/node_task_queue.cc‎

Lines 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
}

‎src/node_worker.cc‎

Lines 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)
Back | FazBrowse Home | New Git URL