Skip to content

Commit 0a35d49

Browse files
addaleaxrichardlau
authored andcommitted
Revert "embedding: make Stop() stop Workers"
This reverts commit 037ac99. 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: #32531 PR-URL: #32623 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>
1 parent 1b4790b commit 0a35d49

File tree

5 files changed

+8
-9
lines changed

5 files changed

+8
-9
lines changed

‎src/api/environment.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,8 @@ ThreadId AllocateEnvironmentThreadId(){
712712
}
713713

714714
voidDefaultProcessExitHandler(Environment* env, int exit_code){
715-
Stop(env);
715+
env->set_can_call_into_js(false);
716+
env->stop_sub_worker_contexts();
716717
DisposePlatform();
717718
uv_library_shutdown();
718719
exit(exit_code);

‎src/env.cc‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -550,10 +550,9 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier){
550550
}
551551
}
552552

553-
voidEnvironment::Stop(){
553+
voidEnvironment::ExitEnv(){
554554
set_can_call_into_js(false);
555555
set_stopping(true);
556-
stop_sub_worker_contexts();
557556
isolate_->TerminateExecution();
558557
SetImmediateThreadsafe([](Environment* env){uv_stop(env->event_loop())});
559558
}

‎src/env.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -926,7 +926,7 @@ class Environment : public MemoryRetainer{
926926
voidRegisterHandleCleanups();
927927
voidCleanupHandles();
928928
voidExit(int code);
929-
voidStop();
929+
voidExitEnv();
930930

931931
// Register clean-up cb to be called on environment destruction.
932932
inlinevoidRegisterHandleCleanup(uv_handle_t* handle,

‎src/node.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1035,7 +1035,7 @@ int Start(int argc, char** argv){
10351035
}
10361036

10371037
intStop(Environment* env){
1038-
env->Stop();
1038+
env->ExitEnv();
10391039
return0;
10401040
}
10411041

‎src/node.h‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,7 @@ class Environment;
218218
NODE_EXTERN intStart(int argc, char* argv[]);
219219

220220
// Tear down Node.js while it is running (there are active handles
221-
// in the loop and / or actively executing JavaScript code). This also stops
222-
// all Workers that may have been started earlier.
221+
// in the loop and / or actively executing JavaScript code).
223222
NODE_EXTERN intStop(Environment* env);
224223

225224
// TODO(addaleax): Officially deprecate this and replace it with something
@@ -469,8 +468,8 @@ NODE_EXTERN void FreeEnvironment(Environment* env);
469468
// It receives the Environment* instance and the exit code as arguments.
470469
// This could e.g. call Stop(env); in order to terminate execution and stop
471470
// the event loop.
472-
// The default handler calls Stop(), disposes of the global V8 platform
473-
//instance, if one is being used, and calls exit().
471+
// The default handler disposes of the global V8 platform instance, if one is
472+
// being used, and calls exit().
474473
NODE_EXTERN voidSetProcessExitHandler(
475474
Environment* env,
476475
std::function<void(Environment*, int)>&& handler);

0 commit comments

Comments
(0)