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 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
7 changes: 7 additions & 0 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ const {
exiting_aliased_Uint32Array,
getHiddenValue,
} = internalBinding('util');
const {
exit_code_symbol: kExitCode,
} = internalBinding('symbols');

setupProcessObject();

Expand Down Expand Up @@ -123,6 +126,10 @@ process._exiting = false;
value = code;
}
validateInteger(value, 'code');
process[kExitCode] = value;
} else {
// unset exit code
process[kExitCode] = code;
}
exitCode = code;
},
Expand Down
39 changes: 13 additions & 26 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,14 @@ Maybe<bool> EmitProcessBeforeExit(Environment* env) {
if (!env->destroy_async_id_list()->empty())
AsyncWrap::DestroyAsyncIdsCallback(env);

HandleScope handle_scope(env->isolate());
Isolate* isolate = env->isolate();
HandleScope handle_scope(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>();

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

return ProcessEmit(env, "beforeExit", exit_code).IsEmpty() ?
Nothing<bool>() : Just(true);
Expand All @@ -67,27 +62,19 @@ Maybe<ExitCode> EmitProcessExitInternal(Environment* env) {
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.
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)) {
const std::optional<int32_t>& exit_code = env->exit_code();
const int no_failure = static_cast<int32_t>(ExitCode::kNoFailure);

if (ProcessEmit(
env, "exit", Integer::New(isolate, exit_code.value_or(no_failure)))
.IsEmpty()) {
return Nothing<ExitCode>();
}

return Just(static_cast<ExitCode>(code));
// Reload exit code, it may be changed by `emit('exit')`
return Just(static_cast<ExitCode>(exit_code.value_or(no_failure)));
}

Maybe<int> EmitProcessExit(Environment* 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 @@ -371,6 +371,14 @@ inline AliasedUint32Array& Environment::exiting() {
return exiting_;
}

inline void Environment::set_exit_code(const std::optional<int32_t> value) {
exit_code_ = value;
}

inline const std::optional<int32_t>& Environment::exit_code() const {
return exit_code_;
}

inline void Environment::set_abort_on_uncaught_exception(bool value) {
options_->abort_on_uncaught_exception = value;
}
Expand Down
5 changes: 5 additions & 0 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
#include <functional>
#include <list>
#include <memory>
#include <optional>
#include <ostream>
#include <set>
#include <string>
Expand Down Expand Up @@ -746,6 +747,9 @@ class Environment : public MemoryRetainer {
inline void set_exiting(bool value);
inline AliasedUint32Array& exiting();

inline void set_exit_code(const std::optional<int32_t> value);
inline const std::optional<int32_t>& exit_code() const;

// This stores whether the --abort-on-uncaught-exception flag was passed
// to Node.
inline bool abort_on_uncaught_exception() const;
Expand Down Expand Up @@ -1102,6 +1106,7 @@ class Environment : public MemoryRetainer {
uint32_t function_id_counter_ = 0;

AliasedUint32Array exiting_;
std::optional<int32_t> exit_code_;

AliasedUint32Array should_abort_on_uncaught_toggle_;
int should_not_abort_scope_counter_ = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/env_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
V(owner_symbol, "owner_symbol") \
V(onpskexchange_symbol, "onpskexchange") \
V(resource_symbol, "resource_symbol") \
V(trigger_async_id_symbol, "trigger_async_id_symbol")
V(trigger_async_id_symbol, "trigger_async_id_symbol") \
V(exit_code_symbol, "exit_code_symbol")
RaisinTen marked this conversation as resolved.
Show resolved Hide resolved

// Strings are per-isolate primitives but Environment proxies them
// for the sake of convenience. Strings should be ASCII-only.
Expand Down Expand Up @@ -114,7 +115,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
12 changes: 3 additions & 9 deletions src/node_errors.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1151,15 +1151,9 @@ 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(static_cast<ExitCode>(env->exit_code().value_or(
static_cast<int32_t>(ExitCode::kGenericUserError))));
}

void TriggerUncaughtException(Isolate* isolate, const v8::TryCatch& try_catch) {
Expand Down
35 changes: 35 additions & 0 deletions src/node_process_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
namespace node {
using v8::Context;
using v8::DEFAULT;
using v8::DontEnum;
using v8::EscapableHandleScope;
using v8::Function;
using v8::FunctionCallbackInfo;
Expand Down Expand Up @@ -73,6 +74,27 @@ static void DebugPortSetter(Local<Name> property,
host_port->set_port(static_cast<int>(port));
}

static void ExitCodeGetter(Local<Name> property,
const PropertyCallbackInfo<Value>& info) {
Environment* env = Environment::GetCurrent(info);
std::optional<int32_t> maybe_exit_code = env->exit_code();
if (maybe_exit_code) {
info.GetReturnValue().Set(*maybe_exit_code);
}
}

static void ExitCodeSetter(Local<Name> property,
Local<Value> value,
const PropertyCallbackInfo<void>& info) {
Environment* env = Environment::GetCurrent(info);
if (value->IsNullOrUndefined()) {
return env->set_exit_code(std::nullopt);
}
env->set_exit_code(
value->Int32Value(env->context())
.FromMaybe(static_cast<int32_t>(ExitCode::kNoFailure)));
}

static void GetParentProcessId(Local<Name> property,
const PropertyCallbackInfo<Value>& info) {
info.GetReturnValue().Set(uv_os_getppid());
Expand Down Expand Up @@ -216,6 +238,17 @@ void PatchProcessObject(const FunctionCallbackInfo<Value>& args) {
env->owns_process_state() ? DebugPortSetter : nullptr,
Local<Value>())
.FromJust());

// process[exit_code_symbol]
CHECK(process
->SetAccessor(context,
RaisinTen marked this conversation as resolved.
Show resolved Hide resolved
env->exit_code_symbol(),
ExitCodeGetter,
ExitCodeSetter,
Local<Value>(),
DEFAULT,
DontEnum)
.FromJust());
}

void RegisterProcessExternalReferences(ExternalReferenceRegistry* registry) {
Expand All @@ -225,6 +258,8 @@ void RegisterProcessExternalReferences(ExternalReferenceRegistry* registry) {
registry->Register(DebugPortGetter);
registry->Register(ProcessTitleSetter);
registry->Register(ProcessTitleGetter);
registry->Register(ExitCodeSetter);
registry->Register(ExitCodeGetter);
}

} // namespace node
Expand Down