diff --git a/src/api/environment.cc b/src/api/environment.cc index 634899faa23a25..02e9991e1522a5 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -355,7 +355,23 @@ Environment* CreateEnvironment(IsolateData* isolate_data, } void FreeEnvironment(Environment* env) { - env->RunCleanup(); + { + // TODO(addaleax): This should maybe rather be in a SealHandleScope. + HandleScope handle_scope(env->isolate()); + Context::Scope context_scope(env->context()); + env->set_stopping(true); + env->stop_sub_worker_contexts(); + env->RunCleanup(); + RunAtExit(env); + } + + // This call needs to be made while the `Environment` is still alive + // because we assume that it is available for async tracking in the + // NodePlatform implementation. + MultiIsolatePlatform* platform = env->isolate_data()->platform(); + if (platform != nullptr) + platform->DrainTasks(env->isolate()); + delete env; } diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 998cf260de2a62..831ba528284a52 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -112,7 +112,8 @@ int NodeMainInstance::Run() { HandleScope handle_scope(isolate_); int exit_code = 0; - std::unique_ptr env = CreateMainEnvironment(&exit_code); + DeleteFnPtr env = + CreateMainEnvironment(&exit_code); CHECK_NOT_NULL(env); Context::Scope context_scope(env->context()); @@ -151,10 +152,7 @@ int NodeMainInstance::Run() { exit_code = EmitExit(env.get()); } - env->set_can_call_into_js(false); - env->stop_sub_worker_contexts(); ResetStdio(); - env->RunCleanup(); // TODO(addaleax): Neither NODE_SHARED_MODE nor HAVE_INSPECTOR really // make sense here. @@ -169,10 +167,6 @@ int NodeMainInstance::Run() { } #endif - RunAtExit(env.get()); - - per_process::v8_platform.DrainVMTasks(isolate_); - #if defined(LEAK_SANITIZER) __lsan_do_leak_check(); #endif @@ -182,8 +176,8 @@ int NodeMainInstance::Run() { // TODO(joyeecheung): align this with the CreateEnvironment exposed in node.h // and the environment creation routine in workers somehow. -std::unique_ptr NodeMainInstance::CreateMainEnvironment( - int* exit_code) { +DeleteFnPtr +NodeMainInstance::CreateMainEnvironment(int* exit_code) { *exit_code = 0; // Reset the exit code to 0 HandleScope handle_scope(isolate_); @@ -208,14 +202,14 @@ std::unique_ptr NodeMainInstance::CreateMainEnvironment( CHECK(!context.IsEmpty()); Context::Scope context_scope(context); - std::unique_ptr env = std::make_unique( + DeleteFnPtr env { new Environment( isolate_data_.get(), context, args_, exec_args_, static_cast(Environment::kIsMainThread | Environment::kOwnsProcessState | - Environment::kOwnsInspector)); + Environment::kOwnsInspector)) }; env->InitializeLibuv(per_process::v8_is_profiling); env->InitializeDiagnostics(); diff --git a/src/node_main_instance.h b/src/node_main_instance.h index 5bc18cb3de63c0..cc9f50b9222de3 100644 --- a/src/node_main_instance.h +++ b/src/node_main_instance.h @@ -61,7 +61,8 @@ class NodeMainInstance { // TODO(joyeecheung): align this with the CreateEnvironment exposed in node.h // and the environment creation routine in workers somehow. - std::unique_ptr CreateMainEnvironment(int* exit_code); + DeleteFnPtr CreateMainEnvironment( + int* exit_code); // If nullptr is returned, the binary is not built with embedded // snapshot. diff --git a/src/node_platform.cc b/src/node_platform.cc index 740ace1fb7895e..7628cf5339cdb3 100644 --- a/src/node_platform.cc +++ b/src/node_platform.cc @@ -421,6 +421,7 @@ void PerIsolatePlatformData::RunForegroundTask(uv_timer_t* handle) { void NodePlatform::DrainTasks(Isolate* isolate) { std::shared_ptr per_isolate = ForNodeIsolate(isolate); + if (!per_isolate) return; do { // Worker tasks aren't associated with an Isolate. @@ -489,12 +490,14 @@ std::shared_ptr NodePlatform::ForNodeIsolate(Isolate* isolate) { Mutex::ScopedLock lock(per_isolate_mutex_); auto data = per_isolate_[isolate]; - CHECK(data.second); + CHECK_NOT_NULL(data.first); return data.second; } bool NodePlatform::FlushForegroundTasks(Isolate* isolate) { - return ForNodeIsolate(isolate)->FlushForegroundTasksInternal(); + std::shared_ptr per_isolate = ForNodeIsolate(isolate); + if (!per_isolate) return false; + return per_isolate->FlushForegroundTasksInternal(); } bool NodePlatform::IdleTasksEnabled(Isolate* isolate) { diff --git a/src/node_worker.cc b/src/node_worker.cc index 6ff1a0afe9915b..1e1877e026c63c 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -270,26 +270,18 @@ void Worker::Run() { auto cleanup_env = OnScopeLeave([&]() { if (!env_) return; env_->set_can_call_into_js(false); - Isolate::DisallowJavascriptExecutionScope disallow_js(isolate_, - Isolate::DisallowJavascriptExecutionScope::THROW_ON_FAILURE); { - Context::Scope context_scope(env_->context()); - { - Mutex::ScopedLock lock(mutex_); - stopped_ = true; - this->env_ = nullptr; - } - env_->set_stopping(true); - env_->stop_sub_worker_contexts(); - env_->RunCleanup(); - RunAtExit(env_.get()); - - // This call needs to be made while the `Environment` is still alive - // because we assume that it is available for async tracking in the - // NodePlatform implementation. - platform_->DrainTasks(isolate_); + Mutex::ScopedLock lock(mutex_); + stopped_ = true; + this->env_ = nullptr; } + + // TODO(addaleax): Try moving DisallowJavascriptExecutionScope into + // FreeEnvironment(). + Isolate::DisallowJavascriptExecutionScope disallow_js(isolate_, + Isolate::DisallowJavascriptExecutionScope::THROW_ON_FAILURE); + env_.reset(); }); if (is_stopped()) return;