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: simplify exit code accesses #45125

Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 15 additions & 9 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,11 @@ const {
validateInteger,
} = require('internal/validators');
const {
exiting_aliased_Uint32Array,
exit_info_private_symbol,
getHiddenValue,
kExitCode,
kExiting,
kHasExitCode,
} = internalBinding('util');

setupProcessObject();
Expand All @@ -90,26 +93,24 @@ setupGlobalProxy();
setupBuffer();

process.domain = null;

// process._exiting and process.exitCode
{
const exitingAliasedUint32Array =
getHiddenValue(process, exiting_aliased_Uint32Array);
const fields = getHiddenValue(process, exit_info_private_symbol);

ObjectDefineProperty(process, '_exiting', {
__proto__: null,
get() {
return exitingAliasedUint32Array[0] === 1;
return fields[kExiting] === 1;
},
set(value) {
exitingAliasedUint32Array[0] = value ? 1 : 0;
fields[kExiting] = value ? 1 : 0;
},
enumerable: true,
configurable: true,
});
}
process._exiting = false;

{
let exitCode;

ObjectDefineProperty(process, 'exitCode', {
__proto__: null,
get() {
Expand All @@ -123,13 +124,18 @@ process._exiting = false;
value = code;
}
validateInteger(value, 'code');
fields[kExitCode] = value;
fields[kHasExitCode] = 1;
} else {
fields[kHasExitCode] = 0;
}
exitCode = code;
},
enumerable: true,
configurable: false,
});
}
process._exiting = false;

// process.config is serialized config.gypi
const nativeModule = internalBinding('builtins');
Expand Down
44 changes: 13 additions & 31 deletions src/api/hooks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ using v8::NewStringType;
using v8::Nothing;
using v8::Object;
using v8::String;
using v8::Value;

void RunAtExit(Environment* env) {
env->RunAtExitCallbacks();
Expand All @@ -36,18 +35,12 @@ Maybe<bool> EmitProcessBeforeExit(Environment* env) {
if (!env->destroy_async_id_list()->empty())
AsyncWrap::DestroyAsyncIdsCallback(env);

HandleScope handle_scope(env->isolate());
Local<Context> context = env->context();
Context::Scope context_scope(context);

Local<Value> exit_code_v;
if (!env->process_object()->Get(context, env->exit_code_string())
.ToLocal(&exit_code_v)) return Nothing<bool>();
Isolate* isolate = env->isolate();
HandleScope handle_scope(isolate);
Context::Scope context_scope(env->context());

Local<Integer> exit_code;
if (!exit_code_v->ToInteger(context).ToLocal(&exit_code)) {
return Nothing<bool>();
}
Local<Integer> exit_code = Integer::New(
isolate, static_cast<int32_t>(env->exit_code(ExitCode::kNoFailure)));

return ProcessEmit(env, "beforeExit", exit_code).IsEmpty() ?
Nothing<bool>() : Just(true);
Expand All @@ -65,29 +58,18 @@ Maybe<ExitCode> EmitProcessExitInternal(Environment* env) {
// process.emit('exit')
Isolate* isolate = env->isolate();
HandleScope handle_scope(isolate);
Local<Context> context = env->context();
Context::Scope context_scope(context);
Local<Object> process_object = env->process_object();

// TODO(addaleax): It might be nice to share process.exitCode via
// getter/setter pairs that pass data directly to the native side, so that we
// don't manually have to read and write JS properties here. These getters
// could use e.g. a typed array for performance.
Context::Scope context_scope(env->context());

env->set_exiting(true);

Local<String> exit_code = env->exit_code_string();
Local<Value> code_v;
int code;
if (!process_object->Get(context, exit_code).ToLocal(&code_v) ||
!code_v->Int32Value(context).To(&code) ||
ProcessEmit(env, "exit", Integer::New(isolate, code)).IsEmpty() ||
// Reload exit code, it may be changed by `emit('exit')`
!process_object->Get(context, exit_code).ToLocal(&code_v) ||
!code_v->Int32Value(context).To(&code)) {
Local<Integer> exit_code = Integer::New(
isolate, static_cast<int32_t>(env->exit_code(ExitCode::kNoFailure)));

if (ProcessEmit(env, "exit", exit_code).IsEmpty()) {
return Nothing<ExitCode>();
}

return Just(static_cast<ExitCode>(code));
// Reload exit code, it may be changed by `emit('exit')`
return Just(env->exit_code(ExitCode::kNoFailure));
}

Maybe<int> EmitProcessExit(Environment* env) {
Expand Down
12 changes: 9 additions & 3 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,11 +364,17 @@ inline bool Environment::force_context_aware() const {
}

inline void Environment::set_exiting(bool value) {
exiting_[0] = value ? 1 : 0;
exit_info_[kExiting] = value ? 1 : 0;
}

inline AliasedUint32Array& Environment::exiting() {
return exiting_;
inline ExitCode Environment::exit_code(const ExitCode default_code) const {
return exit_info_[kHasExitCode] == 0
? default_code
: static_cast<ExitCode>(exit_info_[kExitCode]);
}

inline AliasedInt32Array& Environment::exit_info() {
return exit_info_;
}

inline void Environment::set_abort_on_uncaught_exception(bool value) {
Expand Down
8 changes: 4 additions & 4 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ Environment::Environment(IsolateData* isolate_data,
exec_argv_(exec_args),
argv_(args),
exec_path_(GetExecPath(args)),
exiting_(isolate_, 1, MAYBE_FIELD_PTR(env_info, exiting)),
exit_info_(isolate_, 3, MAYBE_FIELD_PTR(env_info, exit_info)),
RaisinTen marked this conversation as resolved.
Show resolved Hide resolved
should_abort_on_uncaught_toggle_(
isolate_,
1,
Expand Down Expand Up @@ -1596,7 +1596,7 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) {
info.immediate_info = immediate_info_.Serialize(ctx, creator);
info.tick_info = tick_info_.Serialize(ctx, creator);
info.performance_state = performance_state_->Serialize(ctx, creator);
info.exiting = exiting_.Serialize(ctx, creator);
info.exit_info = exit_info_.Serialize(ctx, creator);
info.stream_base_state = stream_base_state_.Serialize(ctx, creator);
info.should_abort_on_uncaught_toggle =
should_abort_on_uncaught_toggle_.Serialize(ctx, creator);
Expand Down Expand Up @@ -1638,7 +1638,7 @@ void Environment::DeserializeProperties(const EnvSerializeInfo* info) {
immediate_info_.Deserialize(ctx);
tick_info_.Deserialize(ctx);
performance_state_->Deserialize(ctx);
exiting_.Deserialize(ctx);
exit_info_.Deserialize(ctx);
stream_base_state_.Deserialize(ctx);
should_abort_on_uncaught_toggle_.Deserialize(ctx);

Expand Down Expand Up @@ -1825,7 +1825,7 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const {
tracker->TrackField("builtins_without_cache", builtins_without_cache);
tracker->TrackField("destroy_async_id_list", destroy_async_id_list_);
tracker->TrackField("exec_argv", exec_argv_);
tracker->TrackField("exiting", exiting_);
tracker->TrackField("exit_info", exit_info_);
tracker->TrackField("should_abort_on_uncaught_toggle",
should_abort_on_uncaught_toggle_);
tracker->TrackField("stream_base_state", stream_base_state_);
Expand Down
19 changes: 12 additions & 7 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -459,8 +459,8 @@ class TickInfo : public MemoryRetainer {
AliasedUint8Array fields_;
};

class TrackingTraceStateObserver :
public v8::TracingController::TraceStateObserver {
class TrackingTraceStateObserver
: public v8::TracingController::TraceStateObserver {
RaisinTen marked this conversation as resolved.
Show resolved Hide resolved
public:
explicit TrackingTraceStateObserver(Environment* env) : env_(env) {}

Expand Down Expand Up @@ -514,7 +514,7 @@ struct EnvSerializeInfo {
TickInfo::SerializeInfo tick_info;
ImmediateInfo::SerializeInfo immediate_info;
performance::PerformanceState::SerializeInfo performance_state;
AliasedBufferIndex exiting;
AliasedBufferIndex exit_info;
AliasedBufferIndex stream_base_state;
AliasedBufferIndex should_abort_on_uncaught_toggle;

Expand Down Expand Up @@ -741,10 +741,12 @@ class Environment : public MemoryRetainer {
inline void set_force_context_aware(bool value);
inline bool force_context_aware() const;

// This is a pseudo-boolean that keeps track of whether the process is
// exiting.
// This contains fields that are a pseudo-boolean that keeps track of whether
// the process is exiting, an integer representing the process exit code, and
// a pseudo-boolean to indicate whether the exit code is undefined.
inline AliasedInt32Array& exit_info();
inline void set_exiting(bool value);
inline AliasedUint32Array& exiting();
inline ExitCode exit_code(const ExitCode default_code) const;

// This stores whether the --abort-on-uncaught-exception flag was passed
// to Node.
Expand Down Expand Up @@ -1036,6 +1038,9 @@ class Environment : public MemoryRetainer {

inline void RemoveHeapSnapshotNearHeapLimitCallback(size_t heap_limit);

// Field identifiers for exit_info_
enum ExitInfoField { kExiting = 0, kExitCode, kHasExitCode };

private:
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
const char* errmsg);
Expand Down Expand Up @@ -1101,7 +1106,7 @@ class Environment : public MemoryRetainer {
uint32_t script_id_counter_ = 0;
uint32_t function_id_counter_ = 0;

AliasedUint32Array exiting_;
AliasedInt32Array exit_info_;

AliasedUint32Array should_abort_on_uncaught_toggle_;
int should_not_abort_scope_counter_ = 0;
Expand Down
3 changes: 1 addition & 2 deletions src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
V(napi_type_tag, "node:napi:type_tag") \
V(napi_wrapper, "node:napi:wrapper") \
V(untransferable_object_private_symbol, "node:untransferableObject") \
V(exiting_aliased_Uint32Array, "node:exiting_aliased_Uint32Array")
V(exit_info_private_symbol, "node:exit_info_private_symbol")

// Symbols are per-isolate primitives but Environment proxies them
// for the sake of convenience.
Expand Down Expand Up @@ -114,7 +114,6 @@
V(errno_string, "errno") \
V(error_string, "error") \
V(exchange_string, "exchange") \
V(exit_code_string, "exitCode") \
V(expire_string, "expire") \
V(exponent_string, "exponent") \
V(exports_string, "exports") \
Expand Down
11 changes: 2 additions & 9 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1151,15 +1151,8 @@ void TriggerUncaughtException(Isolate* isolate,
RunAtExit(env);

// If the global uncaught exception handler sets process.exitCode,
// exit with that code. Otherwise, exit with 1.
Local<String> exit_code = env->exit_code_string();
Local<Value> code;
if (process_object->Get(env->context(), exit_code).ToLocal(&code) &&
code->IsInt32()) {
env->Exit(static_cast<ExitCode>(code.As<Int32>()->Value()));
} else {
env->Exit(ExitCode::kGenericUserError);
}
// exit with that code. Otherwise, exit with `ExitCode::kGenericUserError`.
env->Exit(env->exit_code(ExitCode::kGenericUserError));
}

void TriggerUncaughtException(Isolate* isolate, const v8::TryCatch& try_catch) {
Expand Down
6 changes: 3 additions & 3 deletions src/node_process_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,11 @@ MaybeLocal<Object> CreateProcessObject(Realm* realm) {
return MaybeLocal<Object>();
}

// process[exiting_aliased_Uint32Array]
// process[exit_info_private_symbol]
if (process
->SetPrivate(context,
realm->env()->exiting_aliased_Uint32Array(),
realm->env()->exiting().GetJSArray())
realm->env()->exit_info_private_symbol(),
realm->env()->exit_info().GetJSArray())
.IsNothing()) {
return {};
}
Expand Down
6 changes: 3 additions & 3 deletions src/node_snapshotable.cc
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ std::ostream& operator<<(std::ostream& output, const EnvSerializeInfo& i) {
<< "// -- performance_state begins --\n"
<< i.performance_state << ",\n"
<< "// -- performance_state ends --\n"
<< i.exiting << ", // exiting\n"
<< i.exit_info << ", // exit_info\n"
<< i.stream_base_state << ", // stream_base_state\n"
<< i.should_abort_on_uncaught_toggle
<< ", // should_abort_on_uncaught_toggle\n"
Expand Down Expand Up @@ -736,7 +736,7 @@ EnvSerializeInfo FileReader::Read() {
result.immediate_info = Read<ImmediateInfo::SerializeInfo>();
result.performance_state =
Read<performance::PerformanceState::SerializeInfo>();
result.exiting = Read<AliasedBufferIndex>();
result.exit_info = Read<AliasedBufferIndex>();
result.stream_base_state = Read<AliasedBufferIndex>();
result.should_abort_on_uncaught_toggle = Read<AliasedBufferIndex>();
result.principal_realm = Read<RealmSerializeInfo>();
Expand All @@ -757,7 +757,7 @@ size_t FileWriter::Write(const EnvSerializeInfo& data) {
written_total += Write<ImmediateInfo::SerializeInfo>(data.immediate_info);
written_total += Write<performance::PerformanceState::SerializeInfo>(
data.performance_state);
written_total += Write<AliasedBufferIndex>(data.exiting);
written_total += Write<AliasedBufferIndex>(data.exit_info);
written_total += Write<AliasedBufferIndex>(data.stream_base_state);
written_total +=
Write<AliasedBufferIndex>(data.should_abort_on_uncaught_toggle);
Expand Down
11 changes: 11 additions & 0 deletions src/node_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,17 @@ void Initialize(Local<Object> target,
V(kRejected);
#undef V

#define V(name) \
target \
->Set(context, \
FIXED_ONE_BYTE_STRING(env->isolate(), #name), \
Integer::New(env->isolate(), Environment::ExitInfoField::name)) \
.FromJust()
V(kExiting);
V(kExitCode);
V(kHasExitCode);
#undef V

SetMethodNoSideEffect(context, target, "getHiddenValue", GetHiddenValue);
SetMethod(context, target, "setHiddenValue", SetHiddenValue);
SetMethodNoSideEffect(
Expand Down