v12.18.2 proposal

CHANGELOG.md

@@ -28,7 +28,8 @@ release.
 </tr>
 <tr>
     <td valign="top">
-<b><a href="doc/changelogs/CHANGELOG_V12.md#12.18.1">12.18.1</a></b><br/>
+<b><a href="doc/changelogs/CHANGELOG_V12.md#12.18.2">12.18.2</a></b><br/>
+<a href="doc/changelogs/CHANGELOG_V12.md#12.18.1">12.18.1</a><br/>
 <a href="doc/changelogs/CHANGELOG_V12.md#12.18.0">12.18.0</a><br/>
 <a href="doc/changelogs/CHANGELOG_V12.md#12.17.0">12.17.0</a><br/>
 <a href="doc/changelogs/CHANGELOG_V12.md#12.16.3">12.16.3</a><br/>

common.gypi

@@ -34,7 +34,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.38',
+    'v8_embedder_string': '-node.39',
 
     ##### V8 defaults for Node.js #####
 

deps/v8/src/heap/factory.cc

@@ -1677,8 +1677,8 @@ Handle<Script> Factory::NewScriptWithId(Handle<String> source, int script_id,
   script->set_flags(0);
   script->set_host_defined_options(*empty_fixed_array());
   Handle<WeakArrayList> scripts = script_list();
-  scripts = WeakArrayList::AddToEnd(isolate(), scripts,
-                                    MaybeObjectHandle::Weak(script));
+  scripts = WeakArrayList::Append(isolate(), scripts,
+                                  MaybeObjectHandle::Weak(script));
   heap->set_script_list(*scripts);
   LOG(isolate(), ScriptEvent(Logger::ScriptEventType::kCreate, script_id));
   TRACE_EVENT_OBJECT_CREATED_WITH_ID(
@@ -2112,6 +2112,29 @@ Handle<FixedArray> Factory::CopyFixedArrayAndGrow(Handle<FixedArray> array,
   return CopyArrayAndGrow(array, grow_by, allocation);
 }
 
+Handle<WeakArrayList> Factory::NewUninitializedWeakArrayList(
+    int capacity, AllocationType allocation) {
+  DCHECK_LE(0, capacity);
+  if (capacity == 0) return empty_weak_array_list();
+
+  HeapObject obj = AllocateRawWeakArrayList(capacity, allocation);
+  obj.set_map_after_allocation(*weak_array_list_map(), SKIP_WRITE_BARRIER);
+
+  Handle<WeakArrayList> result(WeakArrayList::cast(obj), isolate());
+  result->set_length(0);
+  result->set_capacity(capacity);
+  return result;
+}
+
+Handle<WeakArrayList> Factory::NewWeakArrayList(int capacity,
+                                                AllocationType allocation) {
+  Handle<WeakArrayList> result =
+      NewUninitializedWeakArrayList(capacity, allocation);
+  MemsetTagged(ObjectSlot(result->data_start()),
+               ReadOnlyRoots(isolate()).undefined_value(), capacity);
+  return result;
+}
+
 Handle<WeakFixedArray> Factory::CopyWeakFixedArrayAndGrow(
     Handle<WeakFixedArray> src, int grow_by, AllocationType allocation) {
   DCHECK(!src->IsTransitionArray());  // Compacted by GC, this code doesn't work
@@ -2123,22 +2146,42 @@ Handle<WeakArrayList> Factory::CopyWeakArrayListAndGrow(
   int old_capacity = src->capacity();
   int new_capacity = old_capacity + grow_by;
   DCHECK_GE(new_capacity, old_capacity);
-  HeapObject obj = AllocateRawWeakArrayList(new_capacity, allocation);
-  obj.set_map_after_allocation(src->map(), SKIP_WRITE_BARRIER);
-
-  WeakArrayList result = WeakArrayList::cast(obj);
+  Handle<WeakArrayList> result =
+      NewUninitializedWeakArrayList(new_capacity, allocation);
   int old_len = src->length();
-  result.set_length(old_len);
-  result.set_capacity(new_capacity);
+  result->set_length(old_len);
 
   // Copy the content.
   DisallowHeapAllocation no_gc;
-  WriteBarrierMode mode = obj.GetWriteBarrierMode(no_gc);
-  result.CopyElements(isolate(), 0, *src, 0, old_len, mode);
-  MemsetTagged(ObjectSlot(result.data_start() + old_len),
+  WriteBarrierMode mode = result->GetWriteBarrierMode(no_gc);
+  result->CopyElements(isolate(), 0, *src, 0, old_len, mode);
+  MemsetTagged(ObjectSlot(result->data_start() + old_len),
                ReadOnlyRoots(isolate()).undefined_value(),
                new_capacity - old_len);
-  return Handle<WeakArrayList>(result, isolate());
+  return result;
+}
+
+Handle<WeakArrayList> Factory::CompactWeakArrayList(Handle<WeakArrayList> src,
+                                                    int new_capacity,
+                                                    AllocationType allocation) {
+  Handle<WeakArrayList> result =
+      NewUninitializedWeakArrayList(new_capacity, allocation);
+
+  // Copy the content.
+  DisallowHeapAllocation no_gc;
+  WriteBarrierMode mode = result->GetWriteBarrierMode(no_gc);
+  int copy_to = 0, length = src->length();
+  for (int i = 0; i < length; i++) {
+    MaybeObject element = src->Get(i);
+    if (element->IsCleared()) continue;
+    result->Set(copy_to++, element, mode);
+  }
+  result->set_length(copy_to);
+
+  MemsetTagged(ObjectSlot(result->data_start() + copy_to),
+               ReadOnlyRoots(isolate()).undefined_value(),
+               new_capacity - copy_to);
+  return result;
 }
 
 Handle<PropertyArray> Factory::CopyPropertyArrayAndGrow(

deps/v8/src/heap/factory.h

@@ -524,6 +524,9 @@ class V8_EXPORT_PRIVATE Factory {
 
   Handle<JSObject> NewFunctionPrototype(Handle<JSFunction> function);
 
+  Handle<WeakArrayList> NewWeakArrayList(
+      int capacity, AllocationType allocation = AllocationType::kYoung);
+
   Handle<WeakCell> NewWeakCell();
 
   // Returns a deep copy of the JavaScript object.
@@ -549,6 +552,10 @@ class V8_EXPORT_PRIVATE Factory {
       Handle<WeakArrayList> array, int grow_by,
       AllocationType allocation = AllocationType::kYoung);
 
+  Handle<WeakArrayList> CompactWeakArrayList(
+      Handle<WeakArrayList> array, int new_capacity,
+      AllocationType allocation = AllocationType::kYoung);
+
   Handle<PropertyArray> CopyPropertyArrayAndGrow(
       Handle<PropertyArray> array, int grow_by,
       AllocationType allocation = AllocationType::kYoung);
@@ -1111,6 +1118,10 @@ class V8_EXPORT_PRIVATE Factory {
   // Initializes JSObject body starting at given offset.
   void InitializeJSObjectBody(Handle<JSObject> obj, Handle<Map> map,
                               int start_offset);
+
+ private:
+  Handle<WeakArrayList> NewUninitializedWeakArrayList(
+      int capacity, AllocationType allocation = AllocationType::kYoung);
 };
 
 // Utility class to simplify argument handling around JSFunction creation.

deps/v8/src/objects/fixed-array.h

@@ -339,6 +339,17 @@ class WeakArrayList : public HeapObject {
       Isolate* isolate, Handle<WeakArrayList> array,
       const MaybeObjectHandle& value);
 
+  // Appends an element to the array and possibly compacts and shrinks live weak
+  // references to the start of the collection. Only use this method when
+  // indices to elements can change.
+  static Handle<WeakArrayList> Append(
+      Isolate* isolate, Handle<WeakArrayList> array,
+      const MaybeObjectHandle& value,
+      AllocationType allocation = AllocationType::kYoung);
+
+  // Compact weak references to the beginning of the array.
+  V8_EXPORT_PRIVATE void Compact(Isolate* isolate);
+
   inline MaybeObject Get(int index) const;
   inline MaybeObject Get(Isolate* isolate, int index) const;
 
@@ -352,6 +363,10 @@ class WeakArrayList : public HeapObject {
     return kHeaderSize + capacity * kTaggedSize;
   }
 
+  static constexpr int CapacityForLength(int length) {
+    return length + Max(length / 2, 2);
+  }
+
   // Gives access to raw memory which stores the array's data.
   inline MaybeObjectSlot data_start();
 
@@ -383,6 +398,9 @@ class WeakArrayList : public HeapObject {
   // Returns the number of non-cleaned weak references in the array.
   int CountLiveWeakReferences() const;
 
+  // Returns the number of non-cleaned elements in the array.
+  int CountLiveElements() const;
+
   // Returns whether an entry was found and removed. Will move the elements
   // around in the array - this method can only be used in cases where the user
   // doesn't care about the indices! Users should make sure there are no

deps/v8/src/objects/objects.cc

@@ -3952,6 +3952,65 @@ Handle<WeakArrayList> WeakArrayList::AddToEnd(Isolate* isolate,
   return array;
 }
 
+// static
+Handle<WeakArrayList> WeakArrayList::Append(Isolate* isolate,
+                                            Handle<WeakArrayList> array,
+                                            const MaybeObjectHandle& value,
+                                            AllocationType allocation) {
+  int length = array->length();
+
+  if (length < array->capacity()) {
+    array->Set(length, *value);
+    array->set_length(length + 1);
+    return array;
+  }
+
+  // Not enough space in the array left, either grow, shrink or
+  // compact the array.
+  int new_length = array->CountLiveElements() + 1;
+
+  bool shrink = new_length < length / 4;
+  bool grow = 3 * (length / 4) < new_length;
+
+  if (shrink || grow) {
+    // Grow or shrink array and compact out-of-place.
+    int new_capacity = CapacityForLength(new_length);
+    array = isolate->factory()->CompactWeakArrayList(array, new_capacity,
+                                                     allocation);
+
+  } else {
+    // Perform compaction in the current array.
+    array->Compact(isolate);
+  }
+
+  // Now append value to the array, there should always be enough space now.
+  DCHECK_LT(array->length(), array->capacity());
+
+  // Reload length, allocation might have killed some weak refs.
+  int index = array->length();
+  array->Set(index, *value);
+  array->set_length(index + 1);
+  return array;
+}
+
+void WeakArrayList::Compact(Isolate* isolate) {
+  int length = this->length();
+  int new_length = 0;
+
+  for (int i = 0; i < length; i++) {
+    MaybeObject value = Get(isolate, i);
+
+    if (!value->IsCleared()) {
+      if (new_length != i) {
+        Set(new_length, value);
+      }
+      ++new_length;
+    }
+  }
+
+  set_length(new_length);
+}
+
 bool WeakArrayList::IsFull() { return length() == capacity(); }
 
 // static
@@ -3961,9 +4020,7 @@ Handle<WeakArrayList> WeakArrayList::EnsureSpace(Isolate* isolate,
                                                  AllocationType allocation) {
   int capacity = array->capacity();
   if (capacity < length) {
-    int new_capacity = length;
-    new_capacity = new_capacity + Max(new_capacity / 2, 2);
-    int grow_by = new_capacity - capacity;
+    int grow_by = CapacityForLength(length) - capacity;
     array = isolate->factory()->CopyWeakArrayListAndGrow(array, grow_by,
                                                          allocation);
   }
@@ -3980,6 +4037,16 @@ int WeakArrayList::CountLiveWeakReferences() const {
   return live_weak_references;
 }
 
+int WeakArrayList::CountLiveElements() const {
+  int non_cleared_objects = 0;
+  for (int i = 0; i < length(); i++) {
+    if (!Get(i)->IsCleared()) {
+      ++non_cleared_objects;
+    }
+  }
+  return non_cleared_objects;
+}
+
 bool WeakArrayList::RemoveOne(const MaybeObjectHandle& value) {
   if (length() == 0) return false;
   // Optimize for the most recently added element to be removed again.

deps/v8/test/unittests/BUILD.gn

@@ -190,6 +190,7 @@ v8_source_set("unittests_sources") {
     "numbers/conversions-unittest.cc",
     "objects/object-unittest.cc",
     "objects/value-serializer-unittest.cc",
+    "objects/weakarraylist-unittest.cc",
     "parser/ast-value-unittest.cc",
     "parser/preparser-unittest.cc",
     "profiler/strings-storage-unittest.cc",

deps/v8/test/unittests/objects/weakarraylist-unittest.cc

@@ -0,0 +1,59 @@
+// Copyright 2019 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "src/heap/factory.h"
+#include "test/unittests/test-utils.h"
+
+namespace v8 {
+namespace internal {
+
+using WeakArrayListTest = TestWithIsolate;
+
+TEST_F(WeakArrayListTest, Compact) {
+  Handle<WeakArrayList> list = isolate()->factory()->NewWeakArrayList(10);
+  EXPECT_EQ(list->length(), 0);
+  EXPECT_EQ(list->capacity(), 10);
+
+  MaybeObject some_object =
+      MaybeObject::FromObject(*isolate()->factory()->empty_fixed_array());
+  MaybeObject weak_ref = MaybeObject::MakeWeak(some_object);
+  MaybeObject smi = MaybeObject::FromSmi(Smi::FromInt(0));
+  MaybeObject cleared_ref = HeapObjectReference::ClearedValue(isolate());
+  list->Set(0, weak_ref);
+  list->Set(1, smi);
+  list->Set(2, cleared_ref);
+  list->Set(3, cleared_ref);
+  list->set_length(5);
+
+  list->Compact(isolate());
+  EXPECT_EQ(list->length(), 3);
+  EXPECT_EQ(list->capacity(), 10);
+}
+
+TEST_F(WeakArrayListTest, OutOfPlaceCompact) {
+  Handle<WeakArrayList> list = isolate()->factory()->NewWeakArrayList(20);
+  EXPECT_EQ(list->length(), 0);
+  EXPECT_EQ(list->capacity(), 20);
+
+  MaybeObject some_object =
+      MaybeObject::FromObject(*isolate()->factory()->empty_fixed_array());
+  MaybeObject weak_ref = MaybeObject::MakeWeak(some_object);
+  MaybeObject smi = MaybeObject::FromSmi(Smi::FromInt(0));
+  MaybeObject cleared_ref = HeapObjectReference::ClearedValue(isolate());
+  list->Set(0, weak_ref);
+  list->Set(1, smi);
+  list->Set(2, cleared_ref);
+  list->Set(3, smi);
+  list->Set(4, cleared_ref);
+  list->set_length(6);
+
+  Handle<WeakArrayList> compacted =
+      isolate()->factory()->CompactWeakArrayList(list, 4);
+  EXPECT_EQ(list->length(), 6);
+  EXPECT_EQ(compacted->length(), 4);
+  EXPECT_EQ(compacted->capacity(), 4);
+}
+
+}  // namespace internal
+}  // namespace v8

doc/changelogs/CHANGELOG_V12.md

@@ -11,6 +11,7 @@
 </tr>
 <tr>
 <td valign="top">
+<a href="#12.18.2">12.18.2</a><br/>
 <a href="#12.18.1">12.18.1</a><br/>
 <a href="#12.18.0">12.18.0</a><br/>
 <a href="#12.17.0">12.17.0</a><br/>
@@ -60,6 +61,21 @@
   * [io.js](CHANGELOG_IOJS.md)
   * [Archive](CHANGELOG_ARCHIVE.md)
 
+<a id="12.18.2"></a>
+## 2020-06-30, Version 12.18.2 'Erbium' (LTS), @BethGriggs
+
+### Notable changes
+
+* **deps**: V8: backport fb26d0bb1835 (Matheus Marchini) [#33573](https://github.com/nodejs/node/pull/33573)
+  * Fixes memory leak in `PrototypeUsers::Add`
+* **src**: use symbol to store `AsyncWrap` resource (Anna Henningsen) [#31745](https://github.com/nodejs/node/pull/31745)
+  * Fixes reported memory leak in [#33468](https://github.com/nodejs/node/issues/33468)
+
+### Commits
+
+* [[`97a3f7b702`](https://github.com/nodejs/node/commit/97a3f7b702)] - **deps**: V8: backport fb26d0bb1835 (Matheus Marchini) [#33573](https://github.com/nodejs/node/pull/33573)
+* [[`30b0339061`](https://github.com/nodejs/node/commit/30b0339061)] - **src**: use symbol to store `AsyncWrap` resource (Anna Henningsen) [#31745](https://github.com/nodejs/node/pull/31745)
+
 <a id="12.18.1"></a>
 ## 2020-06-17, Version 12.18.1 'Erbium' (LTS), @codebytere