Skip to content

Commit 0905663

Browse files
addaleaxnodejs-github-bot
authored andcommitted
vm: add SafeForTerminationScopes for SIGINT interruptions
Some embedders, like Electron, choose to start Node.js with `only_terminate_in_safe_scope` set to `true`. In those cases, parts of the API that expect execution termination to happen need to be marked as able to receive those events. In our case, this is the Ctrl+C support of the `vm` module (and Workers, but since we’re in control of creating the `Isolate` for them, that’s a non-concern there). Add those scopes and add a regression test. PR-URL: #36344 Reviewed-By: Shelley Vohr <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Tobias Nießen <[email protected]> Reviewed-By: Gus Caplan <[email protected]>
1 parent e929d1f commit 0905663

File tree

3 files changed

+61
-0
lines changed

3 files changed

+61
-0
lines changed

src/module_wrap.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,7 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {
350350

351351
ShouldNotAbortOnUncaughtScope no_abort_scope(env);
352352
TryCatchScope try_catch(env);
353+
Isolate::SafeForTerminationScope safe_for_termination(env->isolate());
353354

354355
bool timed_out = false;
355356
bool received_signal = false;

src/node_contextify.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,7 @@ bool ContextifyScript::EvalMachine(Environment* env,
933933
return false;
934934
}
935935
TryCatchScope try_catch(env);
936+
Isolate::SafeForTerminationScope safe_for_termination(env->isolate());
936937
ContextifyScript* wrapped_script;
937938
ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder(), false);
938939
Local<UnboundScript> unbound_script =

test/cctest/test_environment.cc

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,3 +549,62 @@ TEST_F(EnvironmentTest, SetImmediateMicrotasks) {
549549

550550
EXPECT_EQ(called, 1);
551551
}
552+
553+
#ifndef _WIN32 // No SIGINT on Windows.
554+
TEST_F(NodeZeroIsolateTestFixture, CtrlCWithOnlySafeTerminationTest) {
555+
// We need to go through the whole setup dance here because we want to
556+
// set only_terminate_in_safe_scope.
557+
// Allocate and initialize Isolate.
558+
v8::Isolate::CreateParams create_params;
559+
create_params.array_buffer_allocator = allocator.get();
560+
create_params.only_terminate_in_safe_scope = true;
561+
v8::Isolate* isolate = v8::Isolate::Allocate();
562+
CHECK_NOT_NULL(isolate);
563+
platform->RegisterIsolate(isolate, &current_loop);
564+
v8::Isolate::Initialize(isolate, create_params);
565+
566+
// Try creating Context + IsolateData + Environment.
567+
{
568+
v8::Isolate::Scope isolate_scope(isolate);
569+
v8::HandleScope handle_scope(isolate);
570+
571+
auto context = node::NewContext(isolate);
572+
CHECK(!context.IsEmpty());
573+
v8::Context::Scope context_scope(context);
574+
575+
std::unique_ptr<node::IsolateData, decltype(&node::FreeIsolateData)>
576+
isolate_data{node::CreateIsolateData(isolate,
577+
&current_loop,
578+
platform.get()),
579+
node::FreeIsolateData};
580+
CHECK(isolate_data);
581+
582+
std::unique_ptr<node::Environment, decltype(&node::FreeEnvironment)>
583+
environment{node::CreateEnvironment(isolate_data.get(),
584+
context,
585+
{},
586+
{}),
587+
node::FreeEnvironment};
588+
CHECK(environment);
589+
590+
v8::Local<v8::Value> main_ret =
591+
node::LoadEnvironment(environment.get(),
592+
"'use strict';\n"
593+
"const { runInThisContext } = require('vm');\n"
594+
"try {\n"
595+
" runInThisContext("
596+
" `process.kill(process.pid, 'SIGINT'); while(true){}`, "
597+
" { breakOnSigint: true });\n"
598+
" return 'unreachable';\n"
599+
"} catch (err) {\n"
600+
" return err.code;\n"
601+
"}").ToLocalChecked();
602+
node::Utf8Value main_ret_str(isolate, main_ret);
603+
EXPECT_EQ(std::string(*main_ret_str), "ERR_SCRIPT_EXECUTION_INTERRUPTED");
604+
}
605+
606+
// Cleanup.
607+
platform->UnregisterIsolate(isolate);
608+
isolate->Dispose();
609+
}
610+
#endif // _WIN32

0 commit comments

Comments
 (0)