Skip to content

Commit 01bb7b7

Browse files
addaleaxtargos
addaleax
authored and
targos
committed
src: split ownsProcessState off isMainThread
Embedders may want to control whether a Node.js instance controls the current process, similar to what we currently have with `Worker`s. Previously, the `isMainThread` flag had a bit of a double usage, both for indicating whether we are (not) running a Worker and whether we can modify per-process state. PR-URL: #25881 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent fd6ce53 commit 01bb7b7

14 files changed

+76
-41
lines changed

‎lib/internal/bootstrap/node.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414

1515
// This file is compiled as if it's wrapped in a function with arguments
1616
// passed by node::LoadEnvironment()
17-
/* global process, loaderExports, isMainThread */
17+
/* global process, loaderExports, isMainThread, ownsProcessState */
1818

1919
const { internalBinding, NativeModule } = loaderExports;
2020

@@ -51,7 +51,7 @@ const perThreadSetup = NativeModule.require('internal/process/per_thread');
5151
let mainThreadSetup;
5252
// Bootstrappers for the worker threads only
5353
let workerThreadSetup;
54-
if (isMainThread) {
54+
if (ownsProcessState) {
5555
mainThreadSetup = NativeModule.require(
5656
'internal/process/main_thread_only'
5757
);
@@ -140,7 +140,7 @@ if (credentials.implementsPosixCredentials) {
140140
process.getegid = credentials.getegid;
141141
process.getgroups = credentials.getgroups;
142142

143-
if (isMainThread) {
143+
if (ownsProcessState) {
144144
const wrapped = mainThreadSetup.wrapPosixCredentialSetters(credentials);
145145
process.initgroups = wrapped.initgroups;
146146
process.setgroups = wrapped.setgroups;

‎lib/internal/worker.js

+5-3
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,10 @@ const { deserializeError } = require('internal/error-serdes');
3030
const { pathToFileURL } = require('url');
3131

3232
const {
33-
Worker: WorkerImpl,
33+
ownsProcessState,
34+
isMainThread,
3435
threadId,
35-
isMainThread
36+
Worker: WorkerImpl,
3637
} = internalBinding('worker');
3738

3839
const kHandle = Symbol('kHandle');
@@ -251,7 +252,8 @@ function pipeWithoutWarning(source, dest) {
251252
}
252253

253254
module.exports = {
255+
ownsProcessState,
256+
isMainThread,
254257
threadId,
255258
Worker,
256-
isMainThread
257259
};

‎lib/trace_events.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ const {
1313
ERR_INVALID_ARG_TYPE
1414
} = require('internal/errors').codes;
1515

16-
const { isMainThread } = require('internal/worker');
17-
if (!hasTracing || !isMainThread)
16+
const { ownsProcessState } = require('internal/worker');
17+
if (!hasTracing || !ownsProcessState)
1818
throw new ERR_TRACE_EVENTS_UNAVAILABLE();
1919

2020
const { CategorySet, getEnabledCategories } = internalBinding('trace_events');

‎src/api/environment.cc

+6-2
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,12 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
134134
std::vector<std::string> args(argv, argv + argc);
135135
std::vector<std::string> exec_args(exec_argv, exec_argv + exec_argc);
136136
// TODO(addaleax): Provide more sensible flags, in an embedder-accessible way.
137-
Environment* env =
138-
new Environment(isolate_data, context, Environment::kIsMainThread);
137+
Environment* env = new Environment(
138+
isolate_data,
139+
context,
140+
static_cast<Environment::Flags>(Environment::kIsMainThread |
141+
Environment::kOwnsProcessState |
142+
Environment::kOwnsInspector));
139143
env->Start(per_process::v8_is_profiling);
140144
env->ProcessCliArgs(args, exec_args);
141145
return env;

‎src/env-inl.h

+8
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,14 @@ inline bool Environment::is_main_thread() const {
650650
return flags_ & kIsMainThread;
651651
}
652652

653+
inline bool Environment::owns_process_state() const {
654+
return flags_ & kOwnsProcessState;
655+
}
656+
657+
inline bool Environment::owns_inspector() const {
658+
return flags_ & kOwnsInspector;
659+
}
660+
653661
inline uint64_t Environment::thread_id() const {
654662
return thread_id_;
655663
}

‎src/env.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ void InitThreadLocalOnce() {
142142
}
143143

144144
void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() {
145-
if (!env_->is_main_thread()) {
145+
if (!env_->owns_process_state()) {
146146
// Ideally, we’d have a consistent story that treats all threads/Environment
147147
// instances equally here. However, tracing is essentially global, and this
148148
// callback is called from whichever thread calls `StartTracing()` or

‎src/env.h

+5-1
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,9 @@ class Environment {
597597

598598
enum Flags {
599599
kNoFlags = 0,
600-
kIsMainThread = 1
600+
kIsMainThread = 1 << 0,
601+
kOwnsProcessState = 1 << 1,
602+
kOwnsInspector = 1 << 2,
601603
};
602604

603605
static inline Environment* GetCurrent(v8::Isolate* isolate);
@@ -766,6 +768,8 @@ class Environment {
766768
inline void set_has_run_bootstrapping_code(bool has_run_bootstrapping_code);
767769

768770
inline bool is_main_thread() const;
771+
inline bool owns_process_state() const;
772+
inline bool owns_inspector() const;
769773
inline uint64_t thread_id() const;
770774
inline worker::Worker* worker_context() const;
771775
inline void set_worker_context(worker::Worker* context);

‎src/inspector/tracing_agent.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ DispatchResponse TracingAgent::start(
141141
return DispatchResponse::Error(
142142
"Call NodeTracing::end to stop tracing before updating the config");
143143
}
144-
if (!env_->is_main_thread()) {
144+
if (!env_->owns_process_state()) {
145145
return DispatchResponse::Error(
146146
"Tracing properties can only be changed through main thread sessions");
147147
}

‎src/inspector_agent.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,7 @@ bool Agent::Start(const std::string& path,
686686
host_port_ = host_port;
687687

688688
client_ = std::make_shared<NodeInspectorClient>(parent_env_, is_main);
689-
if (parent_env_->is_main_thread()) {
689+
if (parent_env_->owns_inspector()) {
690690
CHECK_EQ(start_io_thread_async_initialized.exchange(true), false);
691691
CHECK_EQ(0, uv_async_init(parent_env_->event_loop(),
692692
&start_io_thread_async,

‎src/node.cc

+9-2
Original file line numberDiff line numberDiff line change
@@ -318,16 +318,18 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
318318
loader_exports_obj->Get(context, env->require_string()).ToLocalChecked();
319319
env->set_native_module_require(require.As<Function>());
320320

321-
// process, loaderExports, isMainThread
321+
// process, loaderExports, isMainThread, ownsProcessState, primordials
322322
std::vector<Local<String>> node_params = {
323323
env->process_string(),
324324
FIXED_ONE_BYTE_STRING(isolate, "loaderExports"),
325325
FIXED_ONE_BYTE_STRING(isolate, "isMainThread"),
326+
FIXED_ONE_BYTE_STRING(isolate, "ownsProcessState"),
326327
env->primordials_string()};
327328
std::vector<Local<Value>> node_args = {
328329
process,
329330
loader_exports_obj,
330331
Boolean::New(isolate, env->is_main_thread()),
332+
Boolean::New(isolate, env->owns_process_state()),
331333
env->primordials()};
332334

333335
MaybeLocal<Value> result = ExecuteBootstrapper(
@@ -756,7 +758,12 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
756758
HandleScope handle_scope(isolate);
757759
Local<Context> context = NewContext(isolate);
758760
Context::Scope context_scope(context);
759-
Environment env(isolate_data, context, Environment::kIsMainThread);
761+
Environment env(
762+
isolate_data,
763+
context,
764+
static_cast<Environment::Flags>(Environment::kIsMainThread |
765+
Environment::kOwnsProcessState |
766+
Environment::kOwnsInspector));
760767
env.Start(per_process::v8_is_profiling);
761768
env.ProcessCliArgs(args, exec_args);
762769

‎src/node_credentials.cc

+5-5
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ static void GetEGid(const FunctionCallbackInfo<Value>& args) {
175175

176176
static void SetGid(const FunctionCallbackInfo<Value>& args) {
177177
Environment* env = Environment::GetCurrent(args);
178-
CHECK(env->is_main_thread());
178+
CHECK(env->owns_process_state());
179179

180180
CHECK_EQ(args.Length(), 1);
181181
CHECK(args[0]->IsUint32() || args[0]->IsString());
@@ -194,7 +194,7 @@ static void SetGid(const FunctionCallbackInfo<Value>& args) {
194194

195195
static void SetEGid(const FunctionCallbackInfo<Value>& args) {
196196
Environment* env = Environment::GetCurrent(args);
197-
CHECK(env->is_main_thread());
197+
CHECK(env->owns_process_state());
198198

199199
CHECK_EQ(args.Length(), 1);
200200
CHECK(args[0]->IsUint32() || args[0]->IsString());
@@ -213,7 +213,7 @@ static void SetEGid(const FunctionCallbackInfo<Value>& args) {
213213

214214
static void SetUid(const FunctionCallbackInfo<Value>& args) {
215215
Environment* env = Environment::GetCurrent(args);
216-
CHECK(env->is_main_thread());
216+
CHECK(env->owns_process_state());
217217

218218
CHECK_EQ(args.Length(), 1);
219219
CHECK(args[0]->IsUint32() || args[0]->IsString());
@@ -232,7 +232,7 @@ static void SetUid(const FunctionCallbackInfo<Value>& args) {
232232

233233
static void SetEUid(const FunctionCallbackInfo<Value>& args) {
234234
Environment* env = Environment::GetCurrent(args);
235-
CHECK(env->is_main_thread());
235+
CHECK(env->owns_process_state());
236236

237237
CHECK_EQ(args.Length(), 1);
238238
CHECK(args[0]->IsUint32() || args[0]->IsString());
@@ -363,7 +363,7 @@ static void Initialize(Local<Object> target,
363363
env->SetMethodNoSideEffect(target, "getegid", GetEGid);
364364
env->SetMethodNoSideEffect(target, "getgroups", GetGroups);
365365

366-
if (env->is_main_thread()) {
366+
if (env->owns_process_state()) {
367367
env->SetMethod(target, "initgroups", InitGroups);
368368
env->SetMethod(target, "setegid", SetEGid);
369369
env->SetMethod(target, "seteuid", SetEUid);

‎src/node_process_methods.cc

+6-6
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ static void Abort(const FunctionCallbackInfo<Value>& args) {
7272

7373
static void Chdir(const FunctionCallbackInfo<Value>& args) {
7474
Environment* env = Environment::GetCurrent(args);
75-
CHECK(env->is_main_thread());
75+
CHECK(env->owns_process_state());
7676

7777
CHECK_EQ(args.Length(), 1);
7878
CHECK(args[0]->IsString());
@@ -392,17 +392,17 @@ static void InitializeProcessMethods(Local<Object> target,
392392
Environment* env = Environment::GetCurrent(context);
393393

394394
// define various internal methods
395-
if (env->is_main_thread()) {
395+
if (env->owns_process_state()) {
396396
env->SetMethod(target, "_debugProcess", DebugProcess);
397397
env->SetMethod(target, "_debugEnd", DebugEnd);
398-
env->SetMethod(
399-
target, "_startProfilerIdleNotifier", StartProfilerIdleNotifier);
400-
env->SetMethod(
401-
target, "_stopProfilerIdleNotifier", StopProfilerIdleNotifier);
402398
env->SetMethod(target, "abort", Abort);
403399
env->SetMethod(target, "chdir", Chdir);
404400
}
405401

402+
env->SetMethod(
403+
target, "_startProfilerIdleNotifier", StartProfilerIdleNotifier);
404+
env->SetMethod(target, "_stopProfilerIdleNotifier", StopProfilerIdleNotifier);
405+
406406
env->SetMethod(target, "umask", Umask);
407407
env->SetMethod(target, "_rawDebug", RawDebug);
408408
env->SetMethod(target, "memoryUsage", MemoryUsage);

‎src/node_process_object.cc

+18-14
Original file line numberDiff line numberDiff line change
@@ -86,15 +86,17 @@ MaybeLocal<Object> CreateProcessObject(
8686

8787
// process.title
8888
auto title_string = FIXED_ONE_BYTE_STRING(env->isolate(), "title");
89-
CHECK(process->SetAccessor(
90-
env->context(),
91-
title_string,
92-
ProcessTitleGetter,
93-
env->is_main_thread() ? ProcessTitleSetter : nullptr,
94-
env->as_external(),
95-
DEFAULT,
96-
None,
97-
SideEffectType::kHasNoSideEffect).FromJust());
89+
CHECK(process
90+
->SetAccessor(
91+
env->context(),
92+
title_string,
93+
ProcessTitleGetter,
94+
env->owns_process_state() ? ProcessTitleSetter : nullptr,
95+
env->as_external(),
96+
DEFAULT,
97+
None,
98+
SideEffectType::kHasNoSideEffect)
99+
.FromJust());
98100

99101
// process.version
100102
READONLY_PROPERTY(process,
@@ -302,11 +304,13 @@ MaybeLocal<Object> CreateProcessObject(
302304

303305
// process.debugPort
304306
auto debug_port_string = FIXED_ONE_BYTE_STRING(env->isolate(), "debugPort");
305-
CHECK(process->SetAccessor(env->context(),
306-
debug_port_string,
307-
DebugPortGetter,
308-
env->is_main_thread() ? DebugPortSetter : nullptr,
309-
env->as_external()).FromJust());
307+
CHECK(process
308+
->SetAccessor(env->context(),
309+
debug_port_string,
310+
DebugPortGetter,
311+
env->owns_process_state() ? DebugPortSetter : nullptr,
312+
env->as_external())
313+
.FromJust());
310314

311315
// process._rawDebug: may be overwritten later in JS land, but should be
312316
// availbale from the begining for debugging purposes

‎src/node_worker.cc

+6
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,12 @@ void InitWorker(Local<Object> target,
569569
FIXED_ONE_BYTE_STRING(env->isolate(), "isMainThread"),
570570
Boolean::New(env->isolate(), env->is_main_thread()))
571571
.FromJust();
572+
573+
target
574+
->Set(env->context(),
575+
FIXED_ONE_BYTE_STRING(env->isolate(), "ownsProcessState"),
576+
Boolean::New(env->isolate(), env->owns_process_state()))
577+
.FromJust();
572578
}
573579

574580
} // anonymous namespace

0 commit comments

Comments
 (0)