src: simplify exit code accesses

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 <[email protected]>
PR-URL: #45125
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Joyee Cheung <[email protected]>

Author: daeyeon

lib/internal/bootstrap/node.js

@@ -80,8 +80,11 @@ const {
   validateInteger,
 } = require('internal/validators');
 const {
-  exiting_aliased_Uint32Array,
+  exit_info_private_symbol,
   getHiddenValue,
+  kExitCode,
+  kExiting,
+  kHasExitCode,
 } = internalBinding('util');
 
 setupProcessObject();
@@ -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() {
@@ -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');

src/api/hooks.cc

@@ -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();
@@ -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);
 }
@@ -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) {

src/env-inl.h

@@ -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) {

src/env.cc

@@ -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,
@@ -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);
@@ -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);
 
@@ -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_);

src/env.h

@@ -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;
 
@@ -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.
@@ -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);
@@ -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;

src/env_properties.h

@@ -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.
@@ -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")                                                 \

src/node_errors.cc

@@ -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) {

src/node_process_object.cc

@@ -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 {};
   }

src/node_snapshotable.cc

@@ -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"
@@ -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>();
@@ -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);

src/node_util.cc

@@ -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(