Skip to content

Commit f621536

Browse files
committed
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 <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
1 parent 7947811 commit f621536

File tree

5 files changed

+8
-9
lines changed

5 files changed

+8
-9
lines changed

src/api/environment.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,8 @@ ThreadId AllocateEnvironmentThreadId() {
725725
}
726726

727727
void DefaultProcessExitHandler(Environment* env, int exit_code) {
728-
Stop(env);
728+
env->set_can_call_into_js(false);
729+
env->stop_sub_worker_contexts();
729730
DisposePlatform();
730731
exit(exit_code);
731732
}

src/env.cc

+1-2
Original file line numberDiff line numberDiff line change
@@ -528,10 +528,9 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {
528528
}
529529
}
530530

531-
void Environment::Stop() {
531+
void Environment::ExitEnv() {
532532
set_can_call_into_js(false);
533533
set_stopping(true);
534-
stop_sub_worker_contexts();
535534
isolate_->TerminateExecution();
536535
SetImmediateThreadsafe([](Environment* env) { uv_stop(env->event_loop()); });
537536
}

src/env.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -899,7 +899,7 @@ class Environment : public MemoryRetainer {
899899
void RegisterHandleCleanups();
900900
void CleanupHandles();
901901
void Exit(int code);
902-
void Stop();
902+
void ExitEnv();
903903

904904
// Register clean-up cb to be called on environment destruction.
905905
inline void RegisterHandleCleanup(uv_handle_t* handle,

src/node.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -1059,7 +1059,7 @@ int Start(int argc, char** argv) {
10591059
}
10601060

10611061
int Stop(Environment* env) {
1062-
env->Stop();
1062+
env->ExitEnv();
10631063
return 0;
10641064
}
10651065

src/node.h

+3-4
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,7 @@ class Environment;
224224
NODE_EXTERN int Start(int argc, char* argv[]);
225225

226226
// Tear down Node.js while it is running (there are active handles
227-
// in the loop and / or actively executing JavaScript code). This also stops
228-
// all Workers that may have been started earlier.
227+
// in the loop and / or actively executing JavaScript code).
229228
NODE_EXTERN int Stop(Environment* env);
230229

231230
// TODO(addaleax): Officially deprecate this and replace it with something
@@ -479,8 +478,8 @@ NODE_EXTERN void FreeEnvironment(Environment* env);
479478
// It receives the Environment* instance and the exit code as arguments.
480479
// This could e.g. call Stop(env); in order to terminate execution and stop
481480
// the event loop.
482-
// The default handler calls Stop(), disposes of the global V8 platform
483-
// instance, if one is being used, and calls exit().
481+
// The default handler disposes of the global V8 platform instance, if one is
482+
// being used, and calls exit().
484483
NODE_EXTERN void SetProcessExitHandler(
485484
Environment* env,
486485
std::function<void(Environment*, int)>&& handler);

0 commit comments

Comments
 (0)