Skip to content

Commit

Permalink
src: simplify exit code accesses
Browse files Browse the repository at this point in the history
This simplifies getting the exit code which is set through
`process.exitCode` by removing manually reading the JS property
from the native side.

Signed-off-by: Daeyeon Jeong <daeyeon.dev@gmail.com>
PR-URL: #45125
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
daeyeon authored Nov 10, 2022
1 parent 09ae62b commit 7b1e153
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 66 deletions.
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
46 changes: 18 additions & 28 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,19 +35,17 @@ 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)) {
if (!env->can_call_into_js()) {
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 +62,22 @@ 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)) {
if (!env->can_call_into_js()) {
return Nothing<ExitCode>();
}

return Just(static_cast<ExitCode>(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>();
}
// 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
9 changes: 5 additions & 4 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -655,7 +655,8 @@ 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_, kExitInfoFieldCount, MAYBE_FIELD_PTR(env_info, exit_info)),
should_abort_on_uncaught_toggle_(
isolate_,
1,
Expand Down Expand Up @@ -1608,7 +1609,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 @@ -1654,7 +1655,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 @@ -1844,7 +1845,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
20 changes: 15 additions & 5 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,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 @@ -743,10 +743,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 @@ -1038,6 +1040,14 @@ class Environment : public MemoryRetainer {

inline void RemoveHeapSnapshotNearHeapLimitCallback(size_t heap_limit);

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

private:
inline void ThrowError(v8::Local<v8::Value> (*fun)(v8::Local<v8::String>),
const char* errmsg);
Expand Down Expand Up @@ -1103,7 +1113,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 @@ -24,7 +24,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

0 comments on commit 7b1e153

Please sign in to comment.