[weakrefs] Clear unregister token-related fields when clearing weak cells

Bug: chromium:1213770
Change-Id: Ic063e79bfa8f3dabdd29d1cc9ed74c7af44d0c31
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2923294Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#74890}

src/heap/mark-compact.cc

@@ -2578,11 +2578,6 @@ void MarkCompactCollector::ClearJSWeakRefs() {
             matched_cell.set_unregister_token(undefined);
           },
           gc_notify_updated_slot);
-      // The following is necessary because in the case that weak_cell has
-      // already been popped and removed from the FinalizationRegistry, the call
-      // to JSFinalizationRegistry::RemoveUnregisterToken above will not find
-      // weak_cell itself to clear its unregister token.
-      weak_cell.set_unregister_token(undefined);
     } else {
       // The unregister_token is alive.
       ObjectSlot slot = weak_cell.RawField(WeakCell::kUnregisterTokenOffset);

src/objects/objects.cc

@@ -6844,6 +6844,7 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap(
       JSFinalizationRegistry::cast(Object(raw_finalization_registry));
   WeakCell weak_cell = WeakCell::cast(Object(raw_weak_cell));
   DCHECK(!weak_cell.unregister_token().IsUndefined(isolate));
+  HeapObject undefined = ReadOnlyRoots(isolate).undefined_value();
 
   // Remove weak_cell from the linked list of other WeakCells with the same
   // unregister token and remove its unregister token from key_map if necessary
@@ -6867,8 +6868,7 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap(
       // of the key in the hash table.
       WeakCell next = WeakCell::cast(weak_cell.key_list_next());
       DCHECK_EQ(next.key_list_prev(), weak_cell);
-      next.set_key_list_prev(ReadOnlyRoots(isolate).undefined_value());
-      weak_cell.set_key_list_next(ReadOnlyRoots(isolate).undefined_value());
+      next.set_key_list_prev(undefined);
       key_map.ValueAtPut(entry, next);
     }
   } else {
@@ -6880,6 +6880,12 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap(
       next.set_key_list_prev(weak_cell.key_list_prev());
     }
   }
+
+  // weak_cell is now removed from the unregister token map, so clear its
+  // unregister token-related fields for heap verification.
+  weak_cell.set_unregister_token(undefined);
+  weak_cell.set_key_list_prev(undefined);
+  weak_cell.set_key_list_next(undefined);
 }
 
 }  // namespace internal

test/cctest/test-js-weak-refs.cc

@@ -979,15 +979,17 @@ TEST(UnregisterTokenHeapVerifier) {
   v8::HandleScope outer_scope(isolate);
 
   {
-    // Make a new FinalizationRegistry and register an object with an unregister
-    // token that's unreachable after the IIFE returns.
+    // Make a new FinalizationRegistry and register two objects with the same
+    // unregister token that's unreachable after the IIFE returns.
     v8::HandleScope scope(isolate);
     CompileRun(
         "var token = {}; "
         "var registry = new FinalizationRegistry(function ()  {}); "
         "(function () { "
-        "  let o = {}; "
-        "  registry.register(o, {}, token); "
+        "  let o1 = {}; "
+        "  let o2 = {}; "
+        "  registry.register(o1, {}, token); "
+        "  registry.register(o2, {}, token); "
         "})();");
   }