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

vm: add SafeForTerminationScopes for SIGINT interruptions #36344

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,7 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo<Value>& args) {

ShouldNotAbortOnUncaughtScope no_abort_scope(env);
TryCatchScope try_catch(env);
Isolate::SafeForTerminationScope safe_for_termination(env->isolate());

bool timed_out = false;
bool received_signal = false;
Expand Down
1 change: 1 addition & 0 deletions src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,7 @@ bool ContextifyScript::EvalMachine(Environment* env,
return false;
}
TryCatchScope try_catch(env);
Isolate::SafeForTerminationScope safe_for_termination(env->isolate());
ContextifyScript* wrapped_script;
ASSIGN_OR_RETURN_UNWRAP(&wrapped_script, args.Holder(), false);
Local<UnboundScript> unbound_script =
Expand Down
59 changes: 59 additions & 0 deletions test/cctest/test_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -549,3 +549,62 @@ TEST_F(EnvironmentTest, SetImmediateMicrotasks) {

EXPECT_EQ(called, 1);
}

#ifndef _WIN32 // No SIGINT on Windows.
TEST_F(NodeZeroIsolateTestFixture, CtrlCWithOnlySafeTerminationTest) {
// We need to go through the whole setup dance here because we want to
// set only_terminate_in_safe_scope.
// Allocate and initialize isolate
v8::Isolate::CreateParams create_params;
create_params.array_buffer_allocator = allocator.get();
create_params.only_terminate_in_safe_scope = true;
v8::Isolate* isolate = v8::Isolate::Allocate();
CHECK_NOT_NULL(isolate);
platform->RegisterIsolate(isolate, &current_loop);
v8::Isolate::Initialize(isolate, create_params);

// Try creating Context + IsolateData + Environment
addaleax marked this conversation as resolved.
Show resolved Hide resolved
{
v8::Isolate::Scope isolate_scope(isolate);
v8::HandleScope handle_scope(isolate);

auto context = node::NewContext(isolate);
CHECK(!context.IsEmpty());
v8::Context::Scope context_scope(context);

std::unique_ptr<node::IsolateData, decltype(&node::FreeIsolateData)>
isolate_data{node::CreateIsolateData(isolate,
&current_loop,
platform.get()),
node::FreeIsolateData};
CHECK(isolate_data);

std::unique_ptr<node::Environment, decltype(&node::FreeEnvironment)>
environment{node::CreateEnvironment(isolate_data.get(),
context,
{},
{}),
node::FreeEnvironment};
CHECK(environment);

v8::Local<v8::Value> main_ret =
node::LoadEnvironment(environment.get(),
"'use strict';\n"
"const { runInThisContext } = require('vm');\n"
"try {\n"
" runInThisContext("
" `process.kill(process.pid, 'SIGINT'); while(true){}`, "
" { breakOnSigint: true });\n"
" return 'unreachable';\n"
"} catch (err) {\n"
" return err.code;\n"
"}").ToLocalChecked();
node::Utf8Value main_ret_str(isolate, main_ret);
EXPECT_EQ(std::string(*main_ret_str), "ERR_SCRIPT_EXECUTION_INTERRUPTED");
}

// Cleanup
platform->UnregisterIsolate(isolate);
isolate->Dispose();
}
#endif // _WIN32