From 4644f38506aaa86167ae5413acc33fbd31ec5e8d Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sun, 31 Jul 2022 12:02:00 +0530 Subject: [PATCH] src: use a typed array internally for process._exiting This would prevent manual writes to the _exiting JS property on the process object by passing the data directly via a typed array for performance. This change partially addresses this TODO: https://github.com/nodejs/node/blob/3d575a4f1bd197c3ce669758a2a3c763462a883a/src/api/hooks.cc#L68-L71 Signed-off-by: Darshan Sen PR-URL: https://github.com/nodejs/node/pull/43883 Reviewed-By: Anna Henningsen Reviewed-By: Joyee Cheung --- lib/internal/bootstrap/node.js | 19 +++++++++++++++++++ src/api/hooks.cc | 13 +++++-------- src/env-inl.h | 8 ++++++++ src/env.cc | 8 ++++++++ src/env.h | 27 ++++++++++++++++++--------- src/node_process_object.cc | 9 +++++++++ typings/internalBinding/util.d.ts | 1 + 7 files changed, 68 insertions(+), 17 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 67cbdb9db09..b02184984c9 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -57,6 +57,10 @@ const { const config = internalBinding('config'); const internalTimers = require('internal/timers'); const { deprecate } = require('internal/util'); +const { + exiting_aliased_Uint32Array, + getHiddenValue, +} = internalBinding('util'); setupProcessObject(); @@ -64,6 +68,21 @@ setupGlobalProxy(); setupBuffer(); process.domain = null; +{ + const exitingAliasedUint32Array = + getHiddenValue(process, exiting_aliased_Uint32Array); + ObjectDefineProperty(process, '_exiting', { + __proto__: null, + get() { + return exitingAliasedUint32Array[0] === 1; + }, + set(value) { + exitingAliasedUint32Array[0] = value ? 1 : 0; + }, + enumerable: true, + configurable: true, + }); +} process._exiting = false; // process.config is serialized config.gypi diff --git a/src/api/hooks.cc b/src/api/hooks.cc index bd26e6d150d..9e54436ba30 100644 --- a/src/api/hooks.cc +++ b/src/api/hooks.cc @@ -65,14 +65,11 @@ Maybe EmitProcessExit(Environment* env) { Context::Scope context_scope(context); Local process_object = env->process_object(); - // TODO(addaleax): It might be nice to share process._exiting and - // 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. - if (process_object - ->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "_exiting"), - True(isolate)).IsNothing()) return Nothing(); + // 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 exit_code = env->exit_code_string(); Local code_v; diff --git a/src/env-inl.h b/src/env-inl.h index 8e2eacd0dd9..97a8bf37629 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -362,6 +362,14 @@ inline bool Environment::force_context_aware() const { return options_->force_context_aware; } +inline void Environment::set_exiting(bool value) { + exiting_[0] = value ? 1 : 0; +} + +inline AliasedUint32Array& Environment::exiting() { + return exiting_; +} + inline void Environment::set_abort_on_uncaught_exception(bool value) { options_->abort_on_uncaught_exception = value; } diff --git a/src/env.cc b/src/env.cc index 5350c0d7cba..26acb9cfb1c 100644 --- a/src/env.cc +++ b/src/env.cc @@ -734,6 +734,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)), should_abort_on_uncaught_toggle_( isolate_, 1, @@ -840,6 +841,9 @@ void Environment::InitializeMainContext(Local context, // By default, always abort when --abort-on-uncaught-exception was passed. should_abort_on_uncaught_toggle_[0] = 1; + // The process is not exiting by default. + set_exiting(false); + performance_state_->Mark(performance::NODE_PERFORMANCE_MILESTONE_ENVIRONMENT, environment_start_time_); performance_state_->Mark(performance::NODE_PERFORMANCE_MILESTONE_NODE_START, @@ -1741,6 +1745,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.stream_base_state = stream_base_state_.Serialize(ctx, creator); info.should_abort_on_uncaught_toggle = should_abort_on_uncaught_toggle_.Serialize(ctx, creator); @@ -1812,6 +1817,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.stream_base_state << ", // stream_base_state\n" << i.should_abort_on_uncaught_toggle << ", // should_abort_on_uncaught_toggle\n" @@ -1858,6 +1864,7 @@ void Environment::DeserializeProperties(const EnvSerializeInfo* info) { immediate_info_.Deserialize(ctx); tick_info_.Deserialize(ctx); performance_state_->Deserialize(ctx); + exiting_.Deserialize(ctx); stream_base_state_.Deserialize(ctx); should_abort_on_uncaught_toggle_.Deserialize(ctx); @@ -2088,6 +2095,7 @@ void Environment::MemoryInfo(MemoryTracker* tracker) const { native_modules_without_cache); tracker->TrackField("destroy_async_id_list", destroy_async_id_list_); tracker->TrackField("exec_argv", exec_argv_); + tracker->TrackField("exiting", exiting_); tracker->TrackField("should_abort_on_uncaught_toggle", should_abort_on_uncaught_toggle_); tracker->TrackField("stream_base_state", stream_base_state_); diff --git a/src/env.h b/src/env.h index 0045ca88b45..242685b4cc4 100644 --- a/src/env.h +++ b/src/env.h @@ -165,15 +165,16 @@ class NoArrayBufferZeroFillScope { // Private symbols are per-isolate primitives but Environment proxies them // for the sake of convenience. Strings should be ASCII-only and have a // "node:" prefix to avoid name clashes with third-party code. -#define PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V) \ - V(alpn_buffer_private_symbol, "node:alpnBuffer") \ - V(arrow_message_private_symbol, "node:arrowMessage") \ - V(contextify_context_private_symbol, "node:contextify:context") \ - V(contextify_global_private_symbol, "node:contextify:global") \ - V(decorated_private_symbol, "node:decorated") \ - V(napi_type_tag, "node:napi:type_tag") \ - V(napi_wrapper, "node:napi:wrapper") \ - V(untransferable_object_private_symbol, "node:untransferableObject") \ +#define PER_ISOLATE_PRIVATE_SYMBOL_PROPERTIES(V) \ + V(alpn_buffer_private_symbol, "node:alpnBuffer") \ + V(arrow_message_private_symbol, "node:arrowMessage") \ + V(contextify_context_private_symbol, "node:contextify:context") \ + V(contextify_global_private_symbol, "node:contextify:global") \ + V(decorated_private_symbol, "node:decorated") \ + 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") // Symbols are per-isolate primitives but Environment proxies them // for the sake of convenience. @@ -953,6 +954,7 @@ struct EnvSerializeInfo { TickInfo::SerializeInfo tick_info; ImmediateInfo::SerializeInfo immediate_info; performance::PerformanceState::SerializeInfo performance_state; + AliasedBufferIndex exiting; AliasedBufferIndex stream_base_state; AliasedBufferIndex should_abort_on_uncaught_toggle; @@ -1128,6 +1130,11 @@ 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. + inline void set_exiting(bool value); + inline AliasedUint32Array& exiting(); + // This stores whether the --abort-on-uncaught-exception flag was passed // to Node. inline bool abort_on_uncaught_exception() const; @@ -1517,6 +1524,8 @@ class Environment : public MemoryRetainer { uint32_t script_id_counter_ = 0; uint32_t function_id_counter_ = 0; + AliasedUint32Array exiting_; + AliasedUint32Array should_abort_on_uncaught_toggle_; int should_not_abort_scope_counter_ = 0; diff --git a/src/node_process_object.cc b/src/node_process_object.cc index 29f6569a45e..3a44c45c6b8 100644 --- a/src/node_process_object.cc +++ b/src/node_process_object.cc @@ -91,6 +91,15 @@ MaybeLocal CreateProcessObject(Environment* env) { return MaybeLocal(); } + // process[exiting_aliased_Uint32Array] + if (process + ->SetPrivate(context, + env->exiting_aliased_Uint32Array(), + env->exiting().GetJSArray()) + .IsNothing()) { + return {}; + } + // process.version READONLY_PROPERTY(process, "version", diff --git a/typings/internalBinding/util.d.ts b/typings/internalBinding/util.d.ts index 40def32d570..bb85acee21a 100644 --- a/typings/internalBinding/util.d.ts +++ b/typings/internalBinding/util.d.ts @@ -17,6 +17,7 @@ declare function InternalBinding(binding: 'util'): { napi_type_tag: 5; napi_wrapper: 6; untransferable_object_private_symbol: 7; + exiting_aliased_Uint32Array: 8; kPending: 0; kFulfilled: 1;