From 2ee344b3318ad28c95e75bc24c260e9b931ffe5a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 11 Jun 2021 20:56:08 +0800 Subject: [PATCH 1/3] bootstrap: split NodeMainInstance::Run() Split the running of the instance so that it can be reused by the snapshot builder when we need to run the loop for user code. --- src/node_main_instance.cc | 26 +++++++++++++------------- src/node_main_instance.h | 1 + 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index b4ceba6adb6020..2320f556e9f453 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -132,21 +132,24 @@ int NodeMainInstance::Run(const EnvSerializeInfo* env_info) { int exit_code = 0; DeleteFnPtr env = CreateMainEnvironment(&exit_code, env_info); - CHECK_NOT_NULL(env); - { - Context::Scope context_scope(env->context()); - if (exit_code == 0) { - LoadEnvironment(env.get(), StartExecutionCallback{}); + Context::Scope context_scope(env->context()); + Run(&exit_code, env.get()); + return exit_code; +} - exit_code = SpinEventLoop(env.get()).FromMaybe(1); - } +void NodeMainInstance::Run(int* exit_code, Environment* env) { + if (*exit_code == 0) { + LoadEnvironment(env, StartExecutionCallback{}); - ResetStdio(); + *exit_code = SpinEventLoop(env).FromMaybe(1); + } - // TODO(addaleax): Neither NODE_SHARED_MODE nor HAVE_INSPECTOR really - // make sense here. + ResetStdio(); + + // TODO(addaleax): Neither NODE_SHARED_MODE nor HAVE_INSPECTOR really + // make sense here. #if HAVE_INSPECTOR && defined(__POSIX__) && !defined(NODE_SHARED_MODE) struct sigaction act; memset(&act, 0, sizeof(act)); @@ -161,9 +164,6 @@ int NodeMainInstance::Run(const EnvSerializeInfo* env_info) { #if defined(LEAK_SANITIZER) __lsan_do_leak_check(); #endif - } - - return exit_code; } DeleteFnPtr diff --git a/src/node_main_instance.h b/src/node_main_instance.h index 75d4b7eac50774..047bdca873ebfd 100644 --- a/src/node_main_instance.h +++ b/src/node_main_instance.h @@ -59,6 +59,7 @@ class NodeMainInstance { // Start running the Node.js instances, return the exit code when finished. int Run(const EnvSerializeInfo* env_info); + void Run(int* exit_code, Environment* env); IsolateData* isolate_data() { return isolate_data_.get(); } From d73a4d89abdb3bd92405c5256d3de0d54a27d58c Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Fri, 11 Jun 2021 21:02:50 +0800 Subject: [PATCH 2/3] bootstrap: move event loop handle checking into snapshot builder This is only necessary for the snapshot builder (because we have no way to resurrect the handles at the moment). In addition, print the handles if the debug flag is set or if the queues are not empty after snapshot is created. --- src/node_main_instance.cc | 2 -- src/node_snapshotable.cc | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 2320f556e9f453..f232cd6a89f26a 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -224,8 +224,6 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code, } } - CHECK(env->req_wrap_queue()->IsEmpty()); - CHECK(env->handle_wrap_queue()->IsEmpty()); return env; } diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index 1871cef443f312..cb3fb6a8a0fdc6 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -157,7 +157,21 @@ void SnapshotBuilder::Generate(SnapshotData* out, // Must be out of HandleScope out->blob = creator.CreateBlob(SnapshotCreator::FunctionCodeHandling::kClear); + + // We must be able to rehash the blob when we restore it or otherwise + // the hash seed would be fixed by V8, introducing a vulnerability. CHECK(out->blob.CanBeRehashed()); + + // We cannot resurrect the handles from the snapshot, so make sure that + // no handles are left open in the environment after the blob is created + // (which should trigger a GC and close all handles that can be closed). + if (!env->req_wrap_queue()->IsEmpty() || !env->handle_wrap_queue()->IsEmpty() + || per_process::enabled_debug_list.enabled(DebugCategory::MKSNAPSHOT)) { + PrintLibuvHandleInformation(env->event_loop(), stderr); + } + CHECK(env->req_wrap_queue()->IsEmpty()); + CHECK(env->handle_wrap_queue()->IsEmpty()); + // Must be done while the snapshot creator isolate is entered i.e. the // creator is still alive. FreeEnvironment(env); From 767278efaa281cda9ce0619e7fde76efb590b9bd Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 15 Jun 2021 23:36:58 +0800 Subject: [PATCH 3/3] fixup! bootstrap: move event loop handle checking into snapshot builder Co-authored-by: Voltrex --- src/node_snapshotable.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index cb3fb6a8a0fdc6..35e0ed3f6df4bf 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -165,7 +165,8 @@ void SnapshotBuilder::Generate(SnapshotData* out, // We cannot resurrect the handles from the snapshot, so make sure that // no handles are left open in the environment after the blob is created // (which should trigger a GC and close all handles that can be closed). - if (!env->req_wrap_queue()->IsEmpty() || !env->handle_wrap_queue()->IsEmpty() + if (!env->req_wrap_queue()->IsEmpty() + || !env->handle_wrap_queue()->IsEmpty() || per_process::enabled_debug_list.enabled(DebugCategory::MKSNAPSHOT)) { PrintLibuvHandleInformation(env->event_loop(), stderr); }