embedding: make Stop() stop Workers

addaleax · addaleax · commit 7a8f59f1d686 · 2020-09-23T20:07:55.000+02:00
This makes sense given that terminating execution of the parent thread this way likely also is supposed to stop all running Worker threads spawned by it. Backport-PR-URL: #35241 PR-URL: #32531 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/src/api/environment.cc b/src/api/environment.cc
@@ -714,8 +714,7 @@ ThreadId AllocateEnvironmentThreadId() {
 }
 
 void DefaultProcessExitHandler(Environment* env, int exit_code) {
-  env->set_can_call_into_js(false);
-  env->stop_sub_worker_contexts();
+  Stop(env);
   DisposePlatform();
   exit(exit_code);
 }
diff --git a/src/env.cc b/src/env.cc
@@ -523,9 +523,10 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {
   }
 }
 
-void Environment::ExitEnv() {
+void Environment::Stop() {
   set_can_call_into_js(false);
   set_stopping(true);
+  stop_sub_worker_contexts();
   isolate_->TerminateExecution();
   SetImmediateThreadsafe([](Environment* env) { uv_stop(env->event_loop()); });
 }
diff --git a/src/env.h b/src/env.h
@@ -913,7 +913,7 @@ class Environment : public MemoryRetainer {
   void RegisterHandleCleanups();
   void CleanupHandles();
   void Exit(int code);
-  void ExitEnv();
+  void Stop();
 
   // 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
@@ -1035,7 +1035,7 @@ int Start(int argc, char** argv) {
 }
 
 int Stop(Environment* env) {
-  env->ExitEnv();
+  env->Stop();
   return 0;
 }
 
diff --git a/src/node.h b/src/node.h
@@ -218,7 +218,8 @@ 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).
+// in the loop and / or actively executing JavaScript code). This also stops
+// all Workers that may have been started earlier.
 NODE_EXTERN int Stop(Environment* env);
 
 // TODO(addaleax): Officially deprecate this and replace it with something
@@ -457,8 +458,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 disposes of the global V8 platform instance, if one is
-// being used, and calls exit().
+// The default handler calls Stop(), 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);

Original file line number	Diff line number	Diff line change
`@@ -714,8 +714,7 @@ ThreadId AllocateEnvironmentThreadId() {`
`714`	`714`	`}`
`715`	`715`
`716`	`716`	`void DefaultProcessExitHandler(Environment* env, int exit_code) {`
`717`		`- env->set_can_call_into_js(false);`
`718`		`- env->stop_sub_worker_contexts();`
	`717`	`+ Stop(env);`
`719`	`718`	`DisposePlatform();`
`720`	`719`	`exit(exit_code);`
`721`	`720`	`}`
Original file line number	Diff line number	Diff line change
`@@ -523,9 +523,10 @@ void Environment::InitializeLibuv(bool start_profiler_idle_notifier) {`
`523`	`523`	`}`
`524`	`524`	`}`
`525`	`525`
`526`		`-void Environment::ExitEnv() {`
	`526`	`+void Environment::Stop() {`
`527`	`527`	`set_can_call_into_js(false);`
`528`	`528`	`set_stopping(true);`
	`529`	`+ stop_sub_worker_contexts();`
`529`	`530`	`isolate_->TerminateExecution();`
`530`	`531`	`SetImmediateThreadsafe([](Environment* env) { uv_stop(env->event_loop()); });`
`531`	`532`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1035,7 +1035,7 @@ int Start(int argc, char** argv) {`
`1035`	`1035`	`}`
`1036`	`1036`
`1037`	`1037`	`int Stop(Environment* env) {`
`1038`		`- env->ExitEnv();`
	`1038`	`+ env->Stop();`
`1039`	`1039`	`return 0;`
`1040`	`1040`	`}`
`1041`	`1041`