Skip to content

Commit

Permalink
src: use a typed array internally for process._exiting
Browse files Browse the repository at this point in the history
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 <raisinten@gmail.com>

PR-URL: #43883
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
RaisinTen authored and juanarbol committed Oct 10, 2022
1 parent 185be29 commit 9b46943
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 17 deletions.
19 changes: 19 additions & 0 deletions lib/internal/bootstrap/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,32 @@ const {
const config = internalBinding('config');
const internalTimers = require('internal/timers');
const { deprecate } = require('internal/util');
const {
exiting_aliased_Uint32Array,
getHiddenValue,
} = internalBinding('util');

setupProcessObject();

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
Expand Down
13 changes: 5 additions & 8 deletions src/api/hooks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,11 @@ Maybe<int> EmitProcessExit(Environment* env) {
Context::Scope context_scope(context);
Local<Object> 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<int>();
// 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;
Expand Down
8 changes: 8 additions & 0 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 8 additions & 0 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -840,6 +841,9 @@ void Environment::InitializeMainContext(Local<Context> 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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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_);
Expand Down
27 changes: 18 additions & 9 deletions src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -951,6 +952,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;

Expand Down Expand Up @@ -1126,6 +1128,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;
Expand Down Expand Up @@ -1515,6 +1522,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;

Expand Down
9 changes: 9 additions & 0 deletions src/node_process_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ MaybeLocal<Object> CreateProcessObject(Environment* env) {
return MaybeLocal<Object>();
}

// process[exiting_aliased_Uint32Array]
if (process
->SetPrivate(context,
env->exiting_aliased_Uint32Array(),
env->exiting().GetJSArray())
.IsNothing()) {
return {};
}

// process.version
READONLY_PROPERTY(process,
"version",
Expand Down
1 change: 1 addition & 0 deletions typings/internalBinding/util.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 9b46943

Please sign in to comment.