src: make AliasedBuffers in the binding data weak

lib/fs.js

@@ -59,9 +59,6 @@ const {
 const pathModule = require('path');
 const { isArrayBufferView } = require('internal/util/types');
 
-// We need to get the statValues from the binding at the callsite since
-// it's re-initialized after deserialization.
-
 const binding = internalBinding('fs');
 
 const { createBlobFromFilePath } = require('internal/blob');
@@ -78,7 +75,10 @@ const {
   uvException,
 } = require('internal/errors');
 
-const { FSReqCallback } = binding;
+const {
+  FSReqCallback,
+  statValues,
+} = binding;
 const { toPathIfFileURL } = require('internal/url');
 const {
   customPromisifyArgs: kCustomPromisifyArgsSymbol,
@@ -2569,8 +2569,8 @@ function realpathSync(p, options) {
 
     // Continue if not a symlink, break if a pipe/socket
     if (knownHard.has(base) || cache?.get(base) === base) {
-      if (isFileType(binding.statValues, S_IFIFO) ||
-          isFileType(binding.statValues, S_IFSOCK)) {
+      if (isFileType(statValues, S_IFIFO) ||
+          isFileType(statValues, S_IFSOCK)) {
         break;
       }
       continue;
@@ -2727,8 +2727,8 @@ function realpath(p, options, callback) {
 
     // Continue if not a symlink, break if a pipe/socket
     if (knownHard.has(base)) {
-      if (isFileType(binding.statValues, S_IFIFO) ||
-          isFileType(binding.statValues, S_IFSOCK)) {
+      if (isFileType(statValues, S_IFIFO) ||
+          isFileType(statValues, S_IFSOCK)) {
         return callback(null, encodeRealpathResult(p, options));
       }
       return process.nextTick(LOOP);

lib/internal/encoding.js

@@ -51,6 +51,7 @@ const {
 const binding = internalBinding('encoding_binding');
 const {
   encodeInto,
+  encodeIntoResults,
   encodeUtf8String,
   decodeUTF8,
 } = binding;
@@ -341,7 +342,7 @@ class TextEncoder {
     encodeInto(src, dest);
     // We need to read from the binding here since the buffer gets refreshed
     // from the snapshot.
-    const { 0: read, 1: written } = binding.encodeIntoResults;
+    const { 0: read, 1: written } = encodeIntoResults;
     return { read, written };
   }
 

lib/v8.js

@@ -139,6 +139,10 @@ const {
   kBytecodeAndMetadataSizeIndex,
   kExternalScriptSourceSizeIndex,
   kCPUProfilerMetaDataSizeIndex,
+
+  heapStatisticsBuffer,
+  heapCodeStatisticsBuffer,
+  heapSpaceStatisticsBuffer,
 } = binding;
 
 const kNumberOfHeapSpaces = kHeapSpaces.length;
@@ -170,7 +174,7 @@ function setFlagsFromString(flags) {
  *   }}
  */
 function getHeapStatistics() {
-  const buffer = binding.heapStatisticsBuffer;
+  const buffer = heapStatisticsBuffer;
 
   updateHeapStatisticsBuffer();
 
@@ -204,7 +208,7 @@ function getHeapStatistics() {
  */
 function getHeapSpaceStatistics() {
   const heapSpaceStatistics = new Array(kNumberOfHeapSpaces);
-  const buffer = binding.heapSpaceStatisticsBuffer;
+  const buffer = heapSpaceStatisticsBuffer;
 
   for (let i = 0; i < kNumberOfHeapSpaces; i++) {
     updateHeapSpaceStatisticsBuffer(i);
@@ -230,7 +234,7 @@ function getHeapSpaceStatistics() {
  *   }}
  */
 function getHeapCodeStatistics() {
-  const buffer = binding.heapCodeStatisticsBuffer;
+  const buffer = heapCodeStatisticsBuffer;
 
   updateHeapCodeStatisticsBuffer();
   return {

src/aliased_buffer-inl.h

@@ -70,14 +70,14 @@ AliasedBufferBase<NativeT, V8T>::AliasedBufferBase(
       count_(that.count_),
       byte_offset_(that.byte_offset_),
       buffer_(that.buffer_) {
-  DCHECK_NULL(index_);
+  DCHECK(is_valid());
   js_array_ = v8::Global<V8T>(that.isolate_, that.GetJSArray());
 }
 
 template <typename NativeT, typename V8T>
 AliasedBufferIndex AliasedBufferBase<NativeT, V8T>::Serialize(
     v8::Local<v8::Context> context, v8::SnapshotCreator* creator) {
-  DCHECK_NULL(index_);
+  DCHECK(is_valid());
   return creator->AddData(context, GetJSArray());
 }
 
@@ -100,7 +100,7 @@ inline void AliasedBufferBase<NativeT, V8T>::Deserialize(
 template <typename NativeT, typename V8T>
 AliasedBufferBase<NativeT, V8T>& AliasedBufferBase<NativeT, V8T>::operator=(
     AliasedBufferBase<NativeT, V8T>&& that) noexcept {
-  DCHECK_NULL(index_);
+  DCHECK(is_valid());
   this->~AliasedBufferBase();
   isolate_ = that.isolate_;
   count_ = that.count_;
@@ -116,7 +116,7 @@ AliasedBufferBase<NativeT, V8T>& AliasedBufferBase<NativeT, V8T>::operator=(
 
 template <typename NativeT, typename V8T>
 v8::Local<V8T> AliasedBufferBase<NativeT, V8T>::GetJSArray() const {
-  DCHECK_NULL(index_);
+  DCHECK(is_valid());
   return js_array_.Get(isolate_);
 }
 
@@ -126,6 +126,21 @@ void AliasedBufferBase<NativeT, V8T>::Release() {
   js_array_.Reset();
 }
 
+template <typename NativeT, typename V8T>
+inline void AliasedBufferBase<NativeT, V8T>::WeakCallback(
+    const v8::WeakCallbackInfo<AliasedBufferBase<NativeT, V8T>>& data) {
+  AliasedBufferBase<NativeT, V8T>* buffer = data.GetParameter();
+  DCHECK(buffer->is_valid());
+  buffer->cleared_ = true;
+  buffer->js_array_.Reset();
+}
+
+template <typename NativeT, typename V8T>
+inline void AliasedBufferBase<NativeT, V8T>::MakeWeak() {
+  DCHECK(is_valid());
+  js_array_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
+}
+
 template <typename NativeT, typename V8T>
 v8::Local<v8::ArrayBuffer> AliasedBufferBase<NativeT, V8T>::GetArrayBuffer()
     const {
@@ -134,7 +149,7 @@ v8::Local<v8::ArrayBuffer> AliasedBufferBase<NativeT, V8T>::GetArrayBuffer()
 
 template <typename NativeT, typename V8T>
 inline const NativeT* AliasedBufferBase<NativeT, V8T>::GetNativeBuffer() const {
-  DCHECK_NULL(index_);
+  DCHECK(is_valid());
   return buffer_;
 }
 
@@ -147,22 +162,22 @@ template <typename NativeT, typename V8T>
 inline void AliasedBufferBase<NativeT, V8T>::SetValue(const size_t index,
                                                       NativeT value) {
   DCHECK_LT(index, count_);
-  DCHECK_NULL(index_);
+  DCHECK(is_valid());
   buffer_[index] = value;
 }
 
 template <typename NativeT, typename V8T>
 inline const NativeT AliasedBufferBase<NativeT, V8T>::GetValue(
     const size_t index) const {
-  DCHECK_NULL(index_);
+  DCHECK(is_valid());
   DCHECK_LT(index, count_);
   return buffer_[index];
 }
 
 template <typename NativeT, typename V8T>
 typename AliasedBufferBase<NativeT, V8T>::Reference
 AliasedBufferBase<NativeT, V8T>::operator[](size_t index) {
-  DCHECK_NULL(index_);
+  DCHECK(is_valid());
   return Reference(this, index);
 }
 
@@ -178,7 +193,7 @@ size_t AliasedBufferBase<NativeT, V8T>::Length() const {
 
 template <typename NativeT, typename V8T>
 void AliasedBufferBase<NativeT, V8T>::reserve(size_t new_capacity) {
-  DCHECK_NULL(index_);
+  DCHECK(is_valid());
   DCHECK_GE(new_capacity, count_);
   DCHECK_EQ(byte_offset_, 0);
   const v8::HandleScope handle_scope(isolate_);
@@ -206,6 +221,11 @@ void AliasedBufferBase<NativeT, V8T>::reserve(size_t new_capacity) {
   count_ = new_capacity;
 }
 
+template <typename NativeT, typename V8T>
+inline bool AliasedBufferBase<NativeT, V8T>::is_valid() const {
+  return index_ == nullptr && !cleared_;
+}
+
 template <typename NativeT, typename V8T>
 inline size_t AliasedBufferBase<NativeT, V8T>::SelfSize() const {
   return sizeof(*this);

src/aliased_buffer.h

@@ -117,6 +117,14 @@ class AliasedBufferBase : public MemoryRetainer {
 
   void Release();
 
+  /**
+   * Make the global reference to the typed array weak. The caller must make
+   * sure that no operation can be done on the AliasedBuffer when the typed
+   * array becomes unreachable. Usually this means the caller must maintain
+   * a JS reference to the typed array from JS object.
+   */
+  inline void MakeWeak();
+
   /**
   *  Get the underlying v8::ArrayBuffer underlying the TypedArray and
   *  overlaying the native buffer
@@ -164,11 +172,15 @@ class AliasedBufferBase : public MemoryRetainer {
   inline void MemoryInfo(node::MemoryTracker* tracker) const override;
 
  private:
+  inline bool is_valid() const;
+  static inline void WeakCallback(
+      const v8::WeakCallbackInfo<AliasedBufferBase<NativeT, V8T>>& data);
   v8::Isolate* isolate_ = nullptr;
   size_t count_ = 0;
   size_t byte_offset_ = 0;
   NativeT* buffer_ = nullptr;
   v8::Global<V8T> js_array_;
+  bool cleared_ = false;
 
   // Deserialize data
   const AliasedBufferIndex* index_ = nullptr;

src/encoding_binding.cc

@@ -28,30 +28,41 @@ void BindingData::MemoryInfo(MemoryTracker* tracker) const {
                       encode_into_results_buffer_);
 }
 
-BindingData::BindingData(Realm* realm, v8::Local<v8::Object> object)
+BindingData::BindingData(Realm* realm,
+                         v8::Local<v8::Object> object,
+                         InternalFieldInfo* info)
     : SnapshotableObject(realm, object, type_int),
-      encode_into_results_buffer_(realm->isolate(), kEncodeIntoResultsLength) {
-  object
-      ->Set(realm->context(),
-            FIXED_ONE_BYTE_STRING(realm->isolate(), "encodeIntoResults"),
-            encode_into_results_buffer_.GetJSArray())
-      .Check();
+      encode_into_results_buffer_(
+          realm->isolate(),
+          kEncodeIntoResultsLength,
+          MAYBE_FIELD_PTR(info, encode_into_results_buffer)) {
+  if (info == nullptr) {
+    object
+        ->Set(realm->context(),
+              FIXED_ONE_BYTE_STRING(realm->isolate(), "encodeIntoResults"),
+              encode_into_results_buffer_.GetJSArray())
+        .Check();
+  } else {
+    encode_into_results_buffer_.Deserialize(realm->context());
+  }
+  encode_into_results_buffer_.MakeWeak();
 }
 
 bool BindingData::PrepareForSerialization(Local<Context> context,
                                           v8::SnapshotCreator* creator) {
-  // We'll just re-initialize the buffers in the constructor since their
-  // contents can be thrown away once consumed in the previous call.
-  encode_into_results_buffer_.Release();
+  DCHECK_NULL(internal_field_info_);
+  internal_field_info_ = InternalFieldInfoBase::New<InternalFieldInfo>(type());
+  internal_field_info_->encode_into_results_buffer =
+      encode_into_results_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_EQ(index, BaseObject::kEmbedderType);
-  InternalFieldInfo* info =
-      InternalFieldInfoBase::New<InternalFieldInfo>(type());
+  InternalFieldInfo* info = internal_field_info_;
+  internal_field_info_ = nullptr;
   return info;
 }
 
@@ -63,7 +74,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>(context, holder);
+  InternalFieldInfo* casted_info = static_cast<InternalFieldInfo*>(info);
+  BindingData* binding =
+      realm->AddBindingData<BindingData>(context, holder, casted_info);
   CHECK_NOT_NULL(binding);
 }
 

src/encoding_binding.h

@@ -14,10 +14,13 @@ class ExternalReferenceRegistry;
 namespace encoding_binding {
 class BindingData : public SnapshotableObject {
  public:
-  BindingData(Realm* realm, v8::Local<v8::Object> obj);
-
-  using InternalFieldInfo = InternalFieldInfoBase;
+  struct InternalFieldInfo : public node::InternalFieldInfoBase {
+    AliasedBufferIndex encode_into_results_buffer;
+  };
 
+  BindingData(Realm* realm,
+              v8::Local<v8::Object> obj,
+              InternalFieldInfo* info = nullptr);
   SERIALIZABLE_OBJECT_METHODS()
   SET_BINDING_ID(encoding_binding_data)
 
@@ -39,6 +42,7 @@ class BindingData : public SnapshotableObject {
  private:
   static constexpr size_t kEncodeIntoResultsLength = 2;
   AliasedUint32Array encode_into_results_buffer_;
+  InternalFieldInfo* internal_field_info_ = nullptr;
 };
 
 }  // namespace encoding_binding