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

src: split ownsProcessState off isMainThread #25881

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 3 additions & 3 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

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

const { internalBinding, NativeModule } = loaderExports;

Expand Down Expand Up @@ -51,7 +51,7 @@ const perThreadSetup = NativeModule.require('internal/process/per_thread');
let mainThreadSetup;
// Bootstrappers for the worker threads only
let workerThreadSetup;
if (isMainThread) {
if (ownsProcessState) {
mainThreadSetup = NativeModule.require(
'internal/process/main_thread_only'
);
Expand Down Expand Up @@ -140,7 +140,7 @@ if (credentials.implementsPosixCredentials) {
process.getegid = credentials.getegid;
process.getgroups = credentials.getgroups;

if (isMainThread) {
if (ownsProcessState) {
const wrapped = mainThreadSetup.wrapPosixCredentialSetters(credentials);
process.initgroups = wrapped.initgroups;
process.setgroups = wrapped.setgroups;
Expand Down
8 changes: 5 additions & 3 deletions lib/internal/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ const { deserializeError } = require('internal/error-serdes');
const { pathToFileURL } = require('url');

const {
Worker: WorkerImpl,
ownsProcessState,
isMainThread,
threadId,
isMainThread
Worker: WorkerImpl,
} = internalBinding('worker');

const kHandle = Symbol('kHandle');
Expand Down Expand Up @@ -243,7 +244,8 @@ function pipeWithoutWarning(source, dest) {
}

module.exports = {
ownsProcessState,
isMainThread,
threadId,
Worker,
isMainThread
};
4 changes: 2 additions & 2 deletions lib/trace_events.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const {
ERR_INVALID_ARG_TYPE
} = require('internal/errors').codes;

const { isMainThread } = require('internal/worker');
if (!hasTracing || !isMainThread)
const { ownsProcessState } = require('internal/worker');
if (!hasTracing || !ownsProcessState)
throw new ERR_TRACE_EVENTS_UNAVAILABLE();

const { CategorySet, getEnabledCategories } = internalBinding('trace_events');
Expand Down
8 changes: 6 additions & 2 deletions src/api/environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,12 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
std::vector<std::string> args(argv, argv + argc);
std::vector<std::string> exec_args(exec_argv, exec_argv + exec_argc);
// TODO(addaleax): Provide more sensible flags, in an embedder-accessible way.
Environment* env =
new Environment(isolate_data, context, Environment::kIsMainThread);
Environment* env = new Environment(
isolate_data,
context,
static_cast<Environment::Flags>(Environment::kIsMainThread |
Environment::kOwnsProcessState |
Environment::kOwnsInspector));
env->Start(per_process::v8_is_profiling);
env->ProcessCliArgs(args, exec_args);
return env;
Expand Down
8 changes: 8 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,14 @@ inline bool Environment::is_main_thread() const {
return flags_ & kIsMainThread;
}

inline bool Environment::owns_process_state() const {
return flags_ & kOwnsProcessState;
}

inline bool Environment::owns_inspector() const {
return flags_ & kOwnsInspector;
}

inline uint64_t Environment::thread_id() const {
return thread_id_;
}
Expand Down
2 changes: 1 addition & 1 deletion src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void InitThreadLocalOnce() {
}

void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() {
if (!env_->is_main_thread()) {
if (!env_->owns_process_state()) {
// Ideally, we’d have a consistent story that treats all threads/Environment
// instances equally here. However, tracing is essentially global, and this
// callback is called from whichever thread calls `StartTracing()` or
Expand Down
6 changes: 5 additions & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,9 @@ class Environment {

enum Flags {
kNoFlags = 0,
kIsMainThread = 1
kIsMainThread = 1 << 0,
kOwnsProcessState = 1 << 1,
kOwnsInspector = 1 << 2,
};

static inline Environment* GetCurrent(v8::Isolate* isolate);
Expand Down Expand Up @@ -768,6 +770,8 @@ class Environment {
inline void set_has_run_bootstrapping_code(bool has_run_bootstrapping_code);

inline bool is_main_thread() const;
inline bool owns_process_state() const;
inline bool owns_inspector() const;
inline uint64_t thread_id() const;
inline worker::Worker* worker_context() const;
inline void set_worker_context(worker::Worker* context);
Expand Down
2 changes: 1 addition & 1 deletion src/inspector/tracing_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ DispatchResponse TracingAgent::start(
return DispatchResponse::Error(
"Call NodeTracing::end to stop tracing before updating the config");
}
if (!env_->is_main_thread()) {
if (!env_->owns_process_state()) {
return DispatchResponse::Error(
"Tracing properties can only be changed through main thread sessions");
}
Expand Down
2 changes: 1 addition & 1 deletion src/inspector_agent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ bool Agent::Start(const std::string& path,
host_port_ = host_port;

client_ = std::make_shared<NodeInspectorClient>(parent_env_, is_main);
if (parent_env_->is_main_thread()) {
if (parent_env_->owns_inspector()) {
CHECK_EQ(start_io_thread_async_initialized.exchange(true), false);
CHECK_EQ(0, uv_async_init(parent_env_->event_loop(),
&start_io_thread_async,
Expand Down
11 changes: 9 additions & 2 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -314,16 +314,18 @@ MaybeLocal<Value> RunBootstrapping(Environment* env) {
loader_exports_obj->Get(context, env->require_string()).ToLocalChecked();
env->set_native_module_require(require.As<Function>());

// process, loaderExports, isMainThread
// process, loaderExports, isMainThread, ownsProcessState, primordials
std::vector<Local<String>> node_params = {
env->process_string(),
FIXED_ONE_BYTE_STRING(isolate, "loaderExports"),
FIXED_ONE_BYTE_STRING(isolate, "isMainThread"),
FIXED_ONE_BYTE_STRING(isolate, "ownsProcessState"),
env->primordials_string()};
std::vector<Local<Value>> node_args = {
process,
loader_exports_obj,
Boolean::New(isolate, env->is_main_thread()),
Boolean::New(isolate, env->owns_process_state()),
env->primordials()};

MaybeLocal<Value> result = ExecuteBootstrapper(
Expand Down Expand Up @@ -752,7 +754,12 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
HandleScope handle_scope(isolate);
Local<Context> context = NewContext(isolate);
Context::Scope context_scope(context);
Environment env(isolate_data, context, Environment::kIsMainThread);
Environment env(
isolate_data,
context,
static_cast<Environment::Flags>(Environment::kIsMainThread |
Environment::kOwnsProcessState |
Environment::kOwnsInspector));
env.Start(per_process::v8_is_profiling);
env.ProcessCliArgs(args, exec_args);

Expand Down
10 changes: 5 additions & 5 deletions src/node_credentials.cc
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ static void GetEGid(const FunctionCallbackInfo<Value>& args) {

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

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

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

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

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

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

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

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

if (env->is_main_thread()) {
if (env->owns_process_state()) {
env->SetMethod(target, "initgroups", InitGroups);
env->SetMethod(target, "setegid", SetEGid);
env->SetMethod(target, "seteuid", SetEUid);
Expand Down
12 changes: 6 additions & 6 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ static void Abort(const FunctionCallbackInfo<Value>& args) {

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

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

// define various internal methods
if (env->is_main_thread()) {
if (env->owns_process_state()) {
env->SetMethod(target, "_debugProcess", DebugProcess);
env->SetMethod(target, "_debugEnd", DebugEnd);
env->SetMethod(
target, "_startProfilerIdleNotifier", StartProfilerIdleNotifier);
env->SetMethod(
target, "_stopProfilerIdleNotifier", StopProfilerIdleNotifier);
env->SetMethod(target, "abort", Abort);
env->SetMethod(target, "chdir", Chdir);
}

env->SetMethod(
target, "_startProfilerIdleNotifier", StartProfilerIdleNotifier);
env->SetMethod(target, "_stopProfilerIdleNotifier", StopProfilerIdleNotifier);

env->SetMethod(target, "umask", Umask);
env->SetMethod(target, "_rawDebug", RawDebug);
env->SetMethod(target, "memoryUsage", MemoryUsage);
Expand Down
32 changes: 18 additions & 14 deletions src/node_process_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -86,15 +86,17 @@ MaybeLocal<Object> CreateProcessObject(

// process.title
auto title_string = FIXED_ONE_BYTE_STRING(env->isolate(), "title");
CHECK(process->SetAccessor(
env->context(),
title_string,
ProcessTitleGetter,
env->is_main_thread() ? ProcessTitleSetter : nullptr,
env->as_external(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect).FromJust());
CHECK(process
->SetAccessor(
env->context(),
title_string,
ProcessTitleGetter,
env->owns_process_state() ? ProcessTitleSetter : nullptr,
env->as_external(),
DEFAULT,
None,
SideEffectType::kHasNoSideEffect)
.FromJust());

// process.version
READONLY_PROPERTY(process,
Expand Down Expand Up @@ -290,11 +292,13 @@ MaybeLocal<Object> CreateProcessObject(

// process.debugPort
auto debug_port_string = FIXED_ONE_BYTE_STRING(env->isolate(), "debugPort");
CHECK(process->SetAccessor(env->context(),
debug_port_string,
DebugPortGetter,
env->is_main_thread() ? DebugPortSetter : nullptr,
env->as_external()).FromJust());
CHECK(process
->SetAccessor(env->context(),
debug_port_string,
DebugPortGetter,
env->owns_process_state() ? DebugPortSetter : nullptr,
env->as_external())
.FromJust());

// process._rawDebug: may be overwritten later in JS land, but should be
// availbale from the begining for debugging purposes
Expand Down
6 changes: 6 additions & 0 deletions src/node_worker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,12 @@ void InitWorker(Local<Object> target,
FIXED_ONE_BYTE_STRING(env->isolate(), "isMainThread"),
Boolean::New(env->isolate(), env->is_main_thread()))
.FromJust();

target
->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "ownsProcessState"),
Boolean::New(env->isolate(), env->owns_process_state()))
.FromJust();
}

} // anonymous namespace
Expand Down