Skip to content

Commit

Permalink
src: use callback scope for main script
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax authored and MylesBorins committed Nov 17, 2019

Verified

This commit was signed with the committer’s verified signature. The key has expired.
addaleax Anna Henningsen
1 parent 8fe6849 commit e3371f0
Showing 7 changed files with 30 additions and 47 deletions.
11 changes: 6 additions & 5 deletions src/api/callback.cc
Original file line number Diff line number Diff line change
@@ -43,12 +43,13 @@ InternalCallbackScope::InternalCallbackScope(AsyncWrap* async_wrap)
InternalCallbackScope::InternalCallbackScope(Environment* env,
Local<Object> object,
const async_context& asyncContext,
ResourceExpectation expect)
int flags)
: env_(env),
async_context_(asyncContext),
object_(object),
callback_scope_(env) {
CHECK_IMPLIES(expect == kRequireResource, !object.IsEmpty());
callback_scope_(env),
skip_hooks_(flags & kSkipAsyncHooks) {
CHECK_IMPLIES(!(flags & kAllowEmptyResource), !object.IsEmpty());
CHECK_NOT_NULL(env);

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

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

if (failed_) return;

if (async_context_.async_id != 0) {
if (async_context_.async_id != 0 && !skip_hooks_) {
AsyncWrap::EmitAfter(env_, async_context_.async_id);
}

9 changes: 2 additions & 7 deletions src/node.cc
Original file line number Diff line number Diff line change
@@ -406,13 +406,8 @@ MaybeLocal<Value> StartExecution(Environment* env, const char* main_script_id) {
->GetFunction(env->context())
.ToLocalChecked()};

Local<Value> result;
if (!ExecuteBootstrapper(env, main_script_id, &parameters, &arguments)
.ToLocal(&result) ||
!task_queue::RunNextTicksNative(env)) {
return MaybeLocal<Value>();
}
return scope.Escape(result);
return scope.EscapeMaybe(
ExecuteBootstrapper(env, main_script_id, &parameters, &arguments));
}

MaybeLocal<Value> StartMainThreadExecution(Environment* env) {
11 changes: 8 additions & 3 deletions src/node_internals.h
Original file line number Diff line number Diff line change
@@ -203,12 +203,16 @@ v8::MaybeLocal<v8::Value> InternalMakeCallback(

class InternalCallbackScope {
public:
// Tell the constructor whether its `object` parameter may be empty or not.
enum ResourceExpectation { kRequireResource, kAllowEmptyResource };
enum Flags {
// Tell the constructor whether its `object` parameter may be empty or not.
kAllowEmptyResource = 1,
// Indicates whether 'before' and 'after' hooks should be skipped.
kSkipAsyncHooks = 2
};
InternalCallbackScope(Environment* env,
v8::Local<v8::Object> object,
const async_context& asyncContext,
ResourceExpectation expect = kRequireResource);
int flags = 0);
// Utility that can be used by AsyncWrap classes.
explicit InternalCallbackScope(AsyncWrap* async_wrap);
~InternalCallbackScope();
@@ -222,6 +226,7 @@ class InternalCallbackScope {
async_context async_context_;
v8::Local<v8::Object> object_;
AsyncCallbackScope callback_scope_;
bool skip_hooks_;
bool failed_ = false;
bool pushed_ids_ = false;
bool closed_ = false;
10 changes: 7 additions & 3 deletions src/node_main_instance.cc
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@ using v8::HandleScope;
using v8::Isolate;
using v8::Local;
using v8::Locker;
using v8::Object;
using v8::SealHandleScope;

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

if (exit_code == 0) {
{
AsyncCallbackScope callback_scope(env.get());
env->async_hooks()->push_async_ids(1, 0);
InternalCallbackScope callback_scope(
env.get(),
Local<Object>(),
{ 1, 0 },
InternalCallbackScope::kAllowEmptyResource |
InternalCallbackScope::kSkipAsyncHooks);
LoadEnvironment(env.get());
env->async_hooks()->pop_async_id(1);
}

env->set_trace_sync_io(env->options()->trace_sync_io);
9 changes: 0 additions & 9 deletions src/node_process.h
Original file line number Diff line number Diff line change
@@ -34,15 +34,6 @@ v8::Maybe<bool> ProcessEmitDeprecationWarning(Environment* env,
v8::MaybeLocal<v8::Object> CreateProcessObject(Environment* env);
void PatchProcessObject(const v8::FunctionCallbackInfo<v8::Value>& args);

namespace task_queue {
// Handle any nextTicks added in the first tick of the program.
// We use the native version here for once so that any microtasks
// created by the main module is then handled from C++, and
// the call stack of the main script does not show up in the async error
// stack trace.
bool RunNextTicksNative(Environment* env);
} // namespace task_queue

} // namespace node
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
#endif // SRC_NODE_PROCESS_H_
16 changes: 0 additions & 16 deletions src/node_task_queue.cc
Original file line number Diff line number Diff line change
@@ -41,22 +41,6 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
isolate->EnqueueMicrotask(args[0].As<Function>());
}

// Should be in sync with runNextTicks in internal/process/task_queues.js
bool RunNextTicksNative(Environment* env) {
OnScopeLeave weakref_cleanup([&]() { env->RunWeakRefCleanup(); });

TickInfo* tick_info = env->tick_info();
if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn())
MicrotasksScope::PerformCheckpoint(env->isolate());
if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn())
return true;

Local<Function> callback = env->tick_callback_function();
CHECK(!callback.IsEmpty());
return !callback->Call(env->context(), env->process_object(), 0, nullptr)
.IsEmpty();
}

static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
MicrotasksScope::PerformCheckpoint(args.GetIsolate());
}
11 changes: 7 additions & 4 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
@@ -347,17 +347,20 @@ void Worker::Run() {
inspector_started = true;
#endif
HandleScope handle_scope(isolate_);
AsyncCallbackScope callback_scope(env_.get());
env_->async_hooks()->push_async_ids(1, 0);
InternalCallbackScope callback_scope(
env_.get(),
Local<Object>(),
{ 1, 0 },
InternalCallbackScope::kAllowEmptyResource |
InternalCallbackScope::kSkipAsyncHooks);

if (!env_->RunBootstrapping().IsEmpty()) {
CreateEnvMessagePort(env_.get());
if (is_stopped()) return;
Debug(this, "Created message port for worker %llu", thread_id_);
USE(StartExecution(env_.get(), "internal/main/worker_thread"));
}

env_->async_hooks()->pop_async_id(1);

Debug(this, "Loaded environment for worker %llu", thread_id_);
}

0 comments on commit e3371f0

Please sign in to comment.