Make JSFinalizationRegistry::next_dirty weak

Currently, JSFinalizationRegistry has a BodyDescriptor that iterates
next_dirty as a custom weak field, and it has a WeakListVisitor that
cleans up any items from the list that should be removed. However, none
of that code is used, because JSFinalizationRegistry objects are created
with visitor ID kVisitJSObjectFast. This change gives them a custom
visitor ID so that next_dirty can be treated as weak.

Bug: v8:12430
Change-Id: I31c1935257ad508b13a3e684662d2ca406d8ed19
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3307096
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78167}

src/heap/concurrent-marking.cc

@@ -127,6 +127,10 @@ class ConcurrentMarkingVisitor final
     return VisitJSObjectSubclass(map, object);
   }
 
+  int VisitJSFinalizationRegistry(Map map, JSFinalizationRegistry object) {
+    return VisitJSObjectSubclass(map, object);
+  }
+
   int VisitConsString(Map map, ConsString object) {
     return VisitFullyWithSnapshot(map, object);
   }
@@ -210,19 +214,21 @@ class ConcurrentMarkingVisitor final
 
     void VisitCodeTarget(Code host, RelocInfo* rinfo) final {
       // This should never happen, because snapshotting is performed only on
-      // JSObjects (and derived classes).
+      // some String subclasses.
       UNREACHABLE();
     }
 
     void VisitEmbeddedPointer(Code host, RelocInfo* rinfo) final {
       // This should never happen, because snapshotting is performed only on
-      // JSObjects (and derived classes).
+      // some String subclasses.
       UNREACHABLE();
     }
 
     void VisitCustomWeakPointers(HeapObject host, ObjectSlot start,
                                  ObjectSlot end) override {
-      DCHECK(host.IsWeakCell() || host.IsJSWeakRef());
+      // This should never happen, because snapshotting is performed only on
+      // some String subclasses.
+      UNREACHABLE();
     }
 
    private:

src/heap/memory-measurement-inl.h

@@ -29,6 +29,7 @@ bool NativeContextInferrer::Infer(Isolate* isolate, Map map, HeapObject object,
                                 native_context);
     case kVisitJSApiObject:
     case kVisitJSArrayBuffer:
+    case kVisitJSFinalizationRegistry:
     case kVisitJSObject:
     case kVisitJSObjectFast:
     case kVisitJSTypedArray:

src/heap/objects-visiting.h

@@ -30,6 +30,7 @@ namespace internal {
   V(FixedDoubleArray)                   \
   V(JSArrayBuffer)                      \
   V(JSDataView)                         \
+  V(JSFinalizationRegistry)             \
   V(JSFunction)                         \
   V(JSObject)                           \
   V(JSTypedArray)                       \

src/objects/map.cc

@@ -249,7 +249,6 @@ VisitorId Map::GetVisitorId(Map map) {
     case JS_CONTEXT_EXTENSION_OBJECT_TYPE:
     case JS_DATE_TYPE:
     case JS_ERROR_TYPE:
-    case JS_FINALIZATION_REGISTRY_TYPE:
     case JS_GENERATOR_OBJECT_TYPE:
     case JS_ITERATOR_PROTOTYPE_TYPE:
     case JS_MAP_ITERATOR_PROTOTYPE_TYPE:
@@ -324,6 +323,9 @@ VisitorId Map::GetVisitorId(Map map) {
     case WEAK_CELL_TYPE:
       return kVisitWeakCell;
 
+    case JS_FINALIZATION_REGISTRY_TYPE:
+      return kVisitJSFinalizationRegistry;
+
     case FILLER_TYPE:
     case FOREIGN_TYPE:
     case HEAP_NUMBER_TYPE:

src/objects/map.h

@@ -46,6 +46,7 @@ enum InstanceType : uint16_t;
   V(JSApiObject)                        \
   V(JSArrayBuffer)                      \
   V(JSDataView)                         \
+  V(JSFinalizationRegistry)             \
   V(JSFunction)                         \
   V(JSObject)                           \
   V(JSObjectFast)                       \

src/profiler/heap-snapshot-generator.cc

@@ -1748,6 +1748,9 @@ bool V8HeapExplorer::IsEssentialHiddenReference(Object parent,
   if (parent.IsContext() &&
       field_offset == Context::OffsetOfElementAt(Context::NEXT_CONTEXT_LINK))
     return false;
+  if (parent.IsJSFinalizationRegistry() &&
+      field_offset == JSFinalizationRegistry::kNextDirtyOffset)
+    return false;
   return true;
 }
 

test/mjsunit/harmony/weakrefs/finalizationregistry-independent-lifetime-multiple.js

@@ -0,0 +1,32 @@
+// Copyright 2021 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.
+
+// Flags: --expose-gc --noincremental-marking --no-concurrent-inlining
+
+let cleanup_called = false;
+function cleanup(holdings) {
+  cleanup_called = true;
+};
+let cleanup_called_2 = false;
+function cleanup2(holdings) {
+  cleanup_called_2 = true;
+};
+let fg = new FinalizationRegistry(cleanup);
+(function() {
+  let fg2 = new FinalizationRegistry(cleanup2);
+  (function() {
+    fg.register({}, {});
+    fg2.register({}, {});
+  })();
+  // Schedule fg and fg2 for cleanup.
+  gc();
+})();
+
+// Collect fg2, but fg is still alive.
+gc();
+
+setTimeout(function() {
+  assertTrue(cleanup_called);
+  assertFalse(cleanup_called_2);
+}, 0);