diff options
author | Anna Henningsen <anna@addaleax.net> | 2020-04-02 23:40:25 +0200 |
---|---|---|
committer | Richard Lau <rlau@redhat.com> | 2021-02-23 11:24:24 +0000 |
commit | 0a35d49f56bdc789ade698595fa4fa09022a28f9 (patch) | |
tree | 83efa5de131dfb77458bb01634f7b8b54a42efcf | |
parent | 1b4790b243fc9616c5c74f056f2100c4f0a20108 (diff) | |
download | node-new-0a35d49f56bdc789ade698595fa4fa09022a28f9.tar.gz |
Revert "embedding: make Stop() stop Workers"
This reverts commit 037ac99ed5aa763b7a3567da2cc81f9d7b97bdf9.
As flaky CI failures have revealed, this feature was implemented
incorrectly. `stop_sub_worker_contexts()` needs to be called on the
thread on which the `Environment` is currently running, it’s not
thread-safe. The current API requires `Stop()` to be thread-safe,
though.
We could add a new API for this, but unless there’s demand, that’s
probably not necessary as `FreeEnvironment()` will also stop Workers,
which is commonly the next action on an `Environment` instance after
having `Stop()` called on it.
Refs: https://github.com/nodejs/node/pull/32531
PR-URL: https://github.com/nodejs/node/pull/32623
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
-rw-r--r-- | src/api/environment.cc | 3 | ||||
-rw-r--r-- | src/env.cc | 3 | ||||
-rw-r--r-- | src/env.h | 2 | ||||
-rw-r--r-- | src/node.cc | 2 | ||||
-rw-r--r-- | src/node.h | 7 |
5 files changed, 8 insertions, 9 deletions
diff --git a/src/api/environment.cc b/src/api/environment.cc index 57d05c963d..b3c9ed72d2 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -712,7 +712,8 @@ ThreadId AllocateEnvironmentThreadId() { } void DefaultProcessExitHandler(Environment* env, int exit_code) { - Stop(env); + env->set_can_call_into_js(false); + env->stop_sub_worker_contexts(); DisposePlatform(); uv_library_shutdown(); exit(exit_code); diff --git a/src/env.cc b/src/env.cc index 4411a46b9c..7e177e3b86 100644 --- a/src/env.cc +++ b/src/env.cc @@ -550,10 +550,9 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) { } } -void Environment::Stop() { +void Environment::ExitEnv() { set_can_call_into_js(false); set_stopping(true); - stop_sub_worker_contexts(); isolate_->TerminateExecution(); SetImmediateThreadsafe([](Environment* env) { uv_stop(env->event_loop()); }); } @@ -926,7 +926,7 @@ class Environment : public MemoryRetainer { void RegisterHandleCleanups(); void CleanupHandles(); void Exit(int code); - void Stop(); + void ExitEnv(); // Register clean-up cb to be called on environment destruction. inline void RegisterHandleCleanup(uv_handle_t* handle, diff --git a/src/node.cc b/src/node.cc index 00ae36cc0f..46e8f74cc2 100644 --- a/src/node.cc +++ b/src/node.cc @@ -1035,7 +1035,7 @@ int Start(int argc, char** argv) { } int Stop(Environment* env) { - env->Stop(); + env->ExitEnv(); return 0; } diff --git a/src/node.h b/src/node.h index 2fda6d125e..e41af9d2c0 100644 --- a/src/node.h +++ b/src/node.h @@ -218,8 +218,7 @@ class Environment; NODE_EXTERN int Start(int argc, char* argv[]); // Tear down Node.js while it is running (there are active handles -// in the loop and / or actively executing JavaScript code). This also stops -// all Workers that may have been started earlier. +// in the loop and / or actively executing JavaScript code). NODE_EXTERN int Stop(Environment* env); // TODO(addaleax): Officially deprecate this and replace it with something @@ -469,8 +468,8 @@ NODE_EXTERN void FreeEnvironment(Environment* env); // It receives the Environment* instance and the exit code as arguments. // This could e.g. call Stop(env); in order to terminate execution and stop // the event loop. -// The default handler calls Stop(), disposes of the global V8 platform -// instance, if one is being used, and calls exit(). +// The default handler disposes of the global V8 platform instance, if one is +// being used, and calls exit(). NODE_EXTERN void SetProcessExitHandler( Environment* env, std::function<void(Environment*, int)>&& handler); |