Skip to content

Commit

Permalink
src: make process binding data weak
Browse files Browse the repository at this point in the history
Avoid the realm being strongly referenced by the process binding data.

PR-URL: #48655
Backport-PR-URL: #51239
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
legendecas authored and UlisesGascon committed Jan 9, 2024
1 parent 9898140 commit 621c4d6
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 33 deletions.
5 changes: 2 additions & 3 deletions lib/internal/process/per_thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const {
StringPrototypeStartsWith,
Symbol,
SymbolIterator,
Uint32Array,
} = primordials;

const {
Expand Down Expand Up @@ -65,10 +64,10 @@ function refreshHrtimeBuffer() {
// The 3 entries filled in by the original process.hrtime contains
// the upper/lower 32 bits of the second part of the value,
// and the remaining nanoseconds of the value.
hrValues = new Uint32Array(binding.hrtimeBuffer);
hrValues = binding.hrtimeBuffer;
// Use a BigUint64Array in the closure because this is actually a bit
// faster than simply returning a BigInt from C++ in V8 7.1.
hrBigintValues = new BigUint64Array(binding.hrtimeBuffer, 0, 1);
hrBigintValues = new BigUint64Array(binding.hrtimeBuffer.buffer, 0, 1);
}

// Create the buffers.
Expand Down
18 changes: 11 additions & 7 deletions src/node_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,20 @@ void PatchProcessObject(const v8::FunctionCallbackInfo<v8::Value>& args);
namespace process {
class BindingData : public SnapshotableObject {
public:
struct InternalFieldInfo : public node::InternalFieldInfoBase {
AliasedBufferIndex hrtime_buffer;
};

static void AddMethods(v8::Isolate* isolate,
v8::Local<v8::ObjectTemplate> target);
static void RegisterExternalReferences(ExternalReferenceRegistry* registry);

using InternalFieldInfo = InternalFieldInfoBase;

SERIALIZABLE_OBJECT_METHODS()
SET_BINDING_ID(process_binding_data)

BindingData(Realm* realm, v8::Local<v8::Object> object);
BindingData(Realm* realm,
v8::Local<v8::Object> object,
InternalFieldInfo* info = nullptr);

void MemoryInfo(MemoryTracker* tracker) const override;
SET_MEMORY_INFO_NAME(BindingData)
Expand All @@ -81,10 +85,10 @@ class BindingData : public SnapshotableObject {
static void SlowBigInt(const v8::FunctionCallbackInfo<v8::Value>& args);

private:
static constexpr size_t kBufferSize =
std::max(sizeof(uint64_t), sizeof(uint32_t) * 3);
v8::Global<v8::ArrayBuffer> array_buffer_;
std::shared_ptr<v8::BackingStore> backing_store_;
// Buffer length in uint32.
static constexpr size_t kHrTimeBufferLength = 3;
AliasedUint32Array hrtime_buffer_;
InternalFieldInfo* internal_field_info_ = nullptr;

// These need to be static so that we have their addresses available to
// register as external references in the snapshot at environment creation
Expand Down
62 changes: 39 additions & 23 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -465,15 +465,29 @@ static void ReallyExit(const FunctionCallbackInfo<Value>& args) {

namespace process {

BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
: SnapshotableObject(realm, object, type_int) {
BindingData::BindingData(Realm* realm,
v8::Local<v8::Object> object,
InternalFieldInfo* info)
: SnapshotableObject(realm, object, type_int),
hrtime_buffer_(realm->isolate(),
kHrTimeBufferLength,
MAYBE_FIELD_PTR(info, hrtime_buffer)) {
Isolate* isolate = realm->isolate();
Local<Context> context = realm->context();
Local<ArrayBuffer> ab = ArrayBuffer::New(isolate, kBufferSize);
array_buffer_.Reset(isolate, ab);
object->Set(context, FIXED_ONE_BYTE_STRING(isolate, "hrtimeBuffer"), ab)
.ToChecked();
backing_store_ = ab->GetBackingStore();

if (info == nullptr) {
object
->Set(context,
FIXED_ONE_BYTE_STRING(isolate, "hrtimeBuffer"),
hrtime_buffer_.GetJSArray())
.ToChecked();
} else {
hrtime_buffer_.Deserialize(realm->context());
}

// The hrtime buffer is referenced from the binding data js object.
// Make the native handle weak to avoid keeping the realm alive.
hrtime_buffer_.MakeWeak();
}

v8::CFunction BindingData::fast_number_(v8::CFunction::Make(FastNumber));
Expand Down Expand Up @@ -503,7 +517,7 @@ BindingData* BindingData::FromV8Value(Local<Value> value) {
}

void BindingData::MemoryInfo(MemoryTracker* tracker) const {
tracker->TrackField("array_buffer", array_buffer_);
tracker->TrackField("hrtime_buffer", hrtime_buffer_);
}

// This is the legacy version of hrtime before BigInt was introduced in
Expand All @@ -516,20 +530,19 @@ void BindingData::MemoryInfo(MemoryTracker* tracker) const {
// because there is no Uint64Array in JS.
// The third entry contains the remaining nanosecond part of the value.
void BindingData::NumberImpl(BindingData* receiver) {
// Make sure we don't accidentally access buffers wiped for snapshot.
CHECK(!receiver->array_buffer_.IsEmpty());
uint64_t t = uv_hrtime();
uint32_t* fields = static_cast<uint32_t*>(receiver->backing_store_->Data());
fields[0] = (t / NANOS_PER_SEC) >> 32;
fields[1] = (t / NANOS_PER_SEC) & 0xffffffff;
fields[2] = t % NANOS_PER_SEC;
receiver->hrtime_buffer_[0] = (t / NANOS_PER_SEC) >> 32;
receiver->hrtime_buffer_[1] = (t / NANOS_PER_SEC) & 0xffffffff;
receiver->hrtime_buffer_[2] = t % NANOS_PER_SEC;
}

void BindingData::BigIntImpl(BindingData* receiver) {
// Make sure we don't accidentally access buffers wiped for snapshot.
CHECK(!receiver->array_buffer_.IsEmpty());
uint64_t t = uv_hrtime();
uint64_t* fields = static_cast<uint64_t*>(receiver->backing_store_->Data());
// The buffer is a Uint32Array, so we need to reinterpret it as a
// Uint64Array to write the value. The buffer is valid at this scope so we
// can safely cast away the constness.
uint64_t* fields = reinterpret_cast<uint64_t*>(
const_cast<uint32_t*>(receiver->hrtime_buffer_.GetNativeBuffer()));
fields[0] = t;
}

Expand All @@ -543,18 +556,19 @@ void BindingData::SlowNumber(const v8::FunctionCallbackInfo<v8::Value>& args) {

bool BindingData::PrepareForSerialization(Local<Context> context,
v8::SnapshotCreator* creator) {
// It's not worth keeping.
// Release it, we will recreate it when the instance is dehydrated.
array_buffer_.Reset();
DCHECK_NULL(internal_field_info_);
internal_field_info_ = InternalFieldInfoBase::New<InternalFieldInfo>(type());
internal_field_info_->hrtime_buffer =
hrtime_buffer_.Serialize(context, creator);
// Return true because we need to maintain the reference to the binding from
// JS land.
return true;
}

InternalFieldInfoBase* BindingData::Serialize(int index) {
DCHECK_IS_SNAPSHOT_SLOT(index);
InternalFieldInfo* info =
InternalFieldInfoBase::New<InternalFieldInfo>(type());
InternalFieldInfo* info = internal_field_info_;
internal_field_info_ = nullptr;
return info;
}

Expand All @@ -566,7 +580,9 @@ void BindingData::Deserialize(Local<Context> context,
v8::HandleScope scope(context->GetIsolate());
Realm* realm = Realm::GetCurrent(context);
// Recreate the buffer in the constructor.
BindingData* binding = realm->AddBindingData<BindingData>(holder);
InternalFieldInfo* casted_info = static_cast<InternalFieldInfo*>(info);
BindingData* binding =
realm->AddBindingData<BindingData>(holder, casted_info);
CHECK_NOT_NULL(binding);
}

Expand Down

0 comments on commit 621c4d6

Please sign in to comment.