From e93503be832ba89748698ac465c34c3f0bcea6ae Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sat, 28 Mar 2020 02:46:41 +0100 Subject: [PATCH] embedding: provide hook for custom process.exit() behaviour Embedders may not want to terminate the process when `process.exit()` is called. This provides a hook for more flexible handling of that situation. Refs: https://github.com/nodejs/node/pull/30467#issuecomment-603689644 PR-URL: https://github.com/nodejs/node/pull/32531 Reviewed-By: Colin Ihrig Reviewed-By: Gireesh Punathil Reviewed-By: James M Snell --- src/api/environment.cc | 13 +++++++++++++ src/env-inl.h | 5 +++++ src/env.cc | 9 +-------- src/env.h | 5 +++++ src/node.h | 12 ++++++++++++ src/node_worker.cc | 10 +++++++--- test/cctest/test_environment.cc | 17 +++++++++++++++++ 7 files changed, 60 insertions(+), 11 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index 128bfc972a0556..e5d4b27e67c8b6 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -724,4 +724,17 @@ ThreadId AllocateEnvironmentThreadId() { return ThreadId { next_thread_id++ }; } +void DefaultProcessExitHandler(Environment* env, int exit_code) { + env->set_can_call_into_js(false); + env->stop_sub_worker_contexts(); + DisposePlatform(); + exit(exit_code); +} + + +void SetProcessExitHandler(Environment* env, + std::function&& handler) { + env->set_process_exit_handler(std::move(handler)); +} + } // namespace node diff --git a/src/env-inl.h b/src/env-inl.h index 9bf57686060b36..dd67974642d1e3 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -1271,6 +1271,11 @@ void Environment::set_main_utf16(std::unique_ptr str) { main_utf16_ = std::move(str); } +void Environment::set_process_exit_handler( + std::function&& handler) { + process_exit_handler_ = std::move(handler); +} + #define VP(PropertyName, StringValue) V(v8::Private, PropertyName) #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName) #define VS(PropertyName, StringValue) V(v8::String, PropertyName) diff --git a/src/env.cc b/src/env.cc index 3b40bd2d191adf..b61501d96bbbd1 100644 --- a/src/env.cc +++ b/src/env.cc @@ -984,14 +984,7 @@ void Environment::Exit(int exit_code) { StackTrace::CurrentStackTrace( isolate(), stack_trace_limit(), StackTrace::kDetailed)); } - if (is_main_thread()) { - set_can_call_into_js(false); - stop_sub_worker_contexts(); - DisposePlatform(); - exit(exit_code); - } else { - worker_context()->Exit(exit_code); - } + process_exit_handler_(this, exit_code); } void Environment::stop_sub_worker_contexts() { diff --git a/src/env.h b/src/env.h index 719aca19a13803..b5630b6ad951ec 100644 --- a/src/env.h +++ b/src/env.h @@ -1255,6 +1255,8 @@ class Environment : public MemoryRetainer { #endif // HAVE_INSPECTOR inline void set_main_utf16(std::unique_ptr); + inline void set_process_exit_handler( + std::function&& handler); private: template @@ -1456,6 +1458,9 @@ class Environment : public MemoryRetainer { int64_t base_object_count_ = 0; std::atomic_bool is_stopping_ { false }; + std::function process_exit_handler_ { + DefaultProcessExitHandler }; + template void ForEachBaseObject(T&& iterator); diff --git a/src/node.h b/src/node.h index 94ef91f5332121..4278eedfcbe09e 100644 --- a/src/node.h +++ b/src/node.h @@ -473,6 +473,18 @@ NODE_EXTERN v8::MaybeLocal LoadEnvironment( std::unique_ptr inspector_parent_handle = {}); NODE_EXTERN void FreeEnvironment(Environment* env); +// Set a callback that is called when process.exit() is called from JS, +// overriding the default handler. +// 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(). +NODE_EXTERN void SetProcessExitHandler( + Environment* env, + std::function&& handler); +NODE_EXTERN void DefaultProcessExitHandler(Environment* env, int exit_code); + // This may return nullptr if context is not associated with a Node instance. NODE_EXTERN Environment* GetCurrentEnvironment(v8::Local context); diff --git a/src/node_worker.cc b/src/node_worker.cc index 26c6ce11cef677..f81daf284dd588 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -311,6 +311,9 @@ void Worker::Run() { if (is_stopped()) return; CHECK_NOT_NULL(env_); env_->set_env_vars(std::move(env_vars_)); + SetProcessExitHandler(env_.get(), [this](Environment*, int exit_code) { + Exit(exit_code); + }); } { Mutex::ScopedLock lock(mutex_); @@ -420,9 +423,10 @@ void Worker::JoinThread() { MakeCallback(env()->onexit_string(), arraysize(args), args); } - // We cleared all libuv handles bound to this Worker above, - // the C++ object is no longer needed for anything now. - MakeWeak(); + // If we get here, the !thread_joined_ condition at the top of the function + // implies that the thread was running. In that case, its final action will + // be to schedule a callback on the parent thread which will delete this + // object, so there's nothing more to do here. } Worker::~Worker() { diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 23b37ee3c60ae4..86dc8f022e0a24 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -447,3 +447,20 @@ TEST_F(EnvironmentTest, InspectorMultipleEmbeddedEnvironments) { CHECK_EQ(from_inspector->IntegerValue(context).FromJust(), 42); } #endif // HAVE_INSPECTOR + +TEST_F(EnvironmentTest, ExitHandlerTest) { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + + int callback_calls = 0; + + Env env {handle_scope, argv}; + SetProcessExitHandler(*env, [&](node::Environment* env_, int exit_code) { + EXPECT_EQ(*env, env_); + EXPECT_EQ(exit_code, 42); + callback_calls++; + node::Stop(*env); + }); + node::LoadEnvironment(*env, "process.exit(42)").ToLocalChecked(); + EXPECT_EQ(callback_calls, 1); +}