Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "embedding: make Stop() stop Workers" #32623

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -725,7 +725,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();
exit(exit_code);
}
Expand Down
5 changes: 3 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -528,10 +528,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()); });
}
Expand Down Expand Up @@ -1015,6 +1014,8 @@ void Environment::Exit(int exit_code) {
}

void Environment::stop_sub_worker_contexts() {
DCHECK_EQ(Isolate::GetCurrent(), isolate());

while (!sub_worker_contexts_.empty()) {
Worker* w = *sub_worker_contexts_.begin();
remove_sub_worker_context(w);
Expand Down
2 changes: 1 addition & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -899,7 +899,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,
Expand Down
2 changes: 1 addition & 1 deletion src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1059,7 +1059,7 @@ int Start(int argc, char** argv) {
}

int Stop(Environment* env) {
env->Stop();
env->ExitEnv();
return 0;
}

Expand Down
7 changes: 3 additions & 4 deletions src/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,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
Expand Down Expand Up @@ -479,8 +478,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);
Expand Down
15 changes: 15 additions & 0 deletions test/parallel/test-worker-terminate-nested.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
'use strict';
const common = require('../common');
const { Worker } = require('worker_threads');

// Check that a Worker that's running another Worker can be terminated.

const worker = new Worker(`
const { Worker, parentPort } = require('worker_threads');
const worker = new Worker('setInterval(() => {}, 10);', { eval: true });
worker.on('online', () => {
parentPort.postMessage({});
});
`, { eval: true });

worker.on('message', common.mustCall(() => worker.terminate()));