Browse Source

chore: cherry-pick 44c4e56fea2c from v8 (#34692)

Jeremy Rose 2 years ago
parent
commit
35f871c702
2 changed files with 186 additions and 0 deletions
  1. 1 0
      patches/v8/.patches
  2. 185 0
      patches/v8/cherry-pick-44c4e56fea2c.patch

+ 1 - 0
patches/v8/.patches

@@ -8,3 +8,4 @@ fix_build_deprecated_attribute_for_older_msvc_versions.patch
 fix_disable_implies_dcheck_for_node_stream_array_buffers.patch
 revert_fix_cppgc_removed_deleted_cstors_in_cppheapcreateparams.patch
 cherry-pick-723ed8a9cfff.patch
+cherry-pick-44c4e56fea2c.patch

+ 185 - 0
patches/v8/cherry-pick-44c4e56fea2c.patch

@@ -0,0 +1,185 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Shu-yu Guo <[email protected]>
+Date: Tue, 3 May 2022 13:26:32 -0700
+Subject: Merged: [weakrefs] Set unregister_token to undefined when
+ unregistering
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+(cherry picked from commit dd3289d7945dac855d1287cf4ea248883e908d54)
+
+Bug: chromium:1321078
+Change-Id: Ic7537cc5101b35018911c27a81e9b0e0a7da154b
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3695262
+Reviewed-by: Dominik Inführ <[email protected]>
+Commit-Queue: Michael Lippautz <[email protected]>
+Cr-Commit-Position: refs/branch-heads/10.2@{#16}
+Cr-Branched-From: 374091f382e88095694c1283cbdc2acddc1b1417-refs/heads/10.2.154@{#1}
+Cr-Branched-From: f0c353f6315eeb2212ba52478983a3b3af07b5b1-refs/heads/main@{#79976}
+
+diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc
+index f1d6755161be21a76d084f2ccb8f73d6976d70f7..6999d52f2d38b1eb937e5fb46d26a90d4bfe6c0f 100644
+--- a/src/heap/mark-compact.cc
++++ b/src/heap/mark-compact.cc
+@@ -3003,14 +3003,11 @@ void MarkCompactCollector::ClearJSWeakRefs() {
+       // unregister_token field set to undefined when processing the first
+       // WeakCell. Like above, we're modifying pointers during GC, so record the
+       // slots.
+-      HeapObject undefined = ReadOnlyRoots(isolate()).undefined_value();
+       JSFinalizationRegistry finalization_registry =
+           JSFinalizationRegistry::cast(weak_cell.finalization_registry());
+       finalization_registry.RemoveUnregisterToken(
+           JSReceiver::cast(unregister_token), isolate(),
+-          [undefined](WeakCell matched_cell) {
+-            matched_cell.set_unregister_token(undefined);
+-          },
++          JSFinalizationRegistry::kKeepMatchedCellsInRegistry,
+           gc_notify_updated_slot);
+     } else {
+       // The unregister_token is alive.
+diff --git a/src/objects/js-weak-refs-inl.h b/src/objects/js-weak-refs-inl.h
+index acce7b72b9430d810b538d6d082e9b57b2a04b75..76e6e075e5ded6bbbf84605c8672b741757eb816 100644
+--- a/src/objects/js-weak-refs-inl.h
++++ b/src/objects/js-weak-refs-inl.h
+@@ -60,16 +60,14 @@ bool JSFinalizationRegistry::Unregister(
+   // key. Each WeakCell will be in the "active_cells" or "cleared_cells" list of
+   // its FinalizationRegistry; remove it from there.
+   return finalization_registry->RemoveUnregisterToken(
+-      *unregister_token, isolate,
+-      [isolate](WeakCell matched_cell) {
+-        matched_cell.RemoveFromFinalizationRegistryCells(isolate);
+-      },
++      *unregister_token, isolate, kRemoveMatchedCellsFromRegistry,
+       [](HeapObject, ObjectSlot, Object) {});
+ }
+ 
+-template <typename MatchCallback, typename GCNotifyUpdatedSlotCallback>
++template <typename GCNotifyUpdatedSlotCallback>
+ bool JSFinalizationRegistry::RemoveUnregisterToken(
+-    JSReceiver unregister_token, Isolate* isolate, MatchCallback match_callback,
++    JSReceiver unregister_token, Isolate* isolate,
++    RemoveUnregisterTokenMode removal_mode,
+     GCNotifyUpdatedSlotCallback gc_notify_updated_slot) {
+   // This method is called from both FinalizationRegistry#unregister and for
+   // removing weakly-held dead unregister tokens. The latter is during GC so
+@@ -107,7 +105,16 @@ bool JSFinalizationRegistry::RemoveUnregisterToken(
+     value = weak_cell.key_list_next();
+     if (weak_cell.unregister_token() == unregister_token) {
+       // weak_cell has the same unregister token; remove it from the key list.
+-      match_callback(weak_cell);
++      switch (removal_mode) {
++        case kRemoveMatchedCellsFromRegistry:
++          weak_cell.RemoveFromFinalizationRegistryCells(isolate);
++          break;
++        case kKeepMatchedCellsInRegistry:
++          // Do nothing.
++          break;
++      }
++      // Clear unregister token-related fields.
++      weak_cell.set_unregister_token(undefined);
+       weak_cell.set_key_list_prev(undefined);
+       weak_cell.set_key_list_next(undefined);
+       was_present = true;
+diff --git a/src/objects/js-weak-refs.h b/src/objects/js-weak-refs.h
+index 57f765b282e653122fd83e827bdd52cb9fe818cf..f678234ff81afc7a32c93bb275edd39fbe0b3891 100644
+--- a/src/objects/js-weak-refs.h
++++ b/src/objects/js-weak-refs.h
+@@ -43,10 +43,14 @@ class JSFinalizationRegistry
+   // it modifies slots in key_map and WeakCells and the normal write barrier is
+   // disabled during GC, we need to tell the GC about the modified slots via the
+   // gc_notify_updated_slot function.
+-  template <typename MatchCallback, typename GCNotifyUpdatedSlotCallback>
++  enum RemoveUnregisterTokenMode {
++    kRemoveMatchedCellsFromRegistry,
++    kKeepMatchedCellsInRegistry
++  };
++  template <typename GCNotifyUpdatedSlotCallback>
+   inline bool RemoveUnregisterToken(
+       JSReceiver unregister_token, Isolate* isolate,
+-      MatchCallback match_callback,
++      RemoveUnregisterTokenMode removal_mode,
+       GCNotifyUpdatedSlotCallback gc_notify_updated_slot);
+ 
+   // Returns true if the cleared_cells list is non-empty.
+diff --git a/src/objects/objects.cc b/src/objects/objects.cc
+index c2c9fe36c57603425dddbf36823489c59bcc4868..0908eb76b3610e6bdb5e7e816773a53d180f3ecf 100644
+--- a/src/objects/objects.cc
++++ b/src/objects/objects.cc
+@@ -6936,7 +6936,7 @@ void JSFinalizationRegistry::RemoveCellFromUnregisterTokenMap(
+   }
+ 
+   // weak_cell is now removed from the unregister token map, so clear its
+-  // unregister token-related fields for heap verification.
++  // unregister token-related fields.
+   weak_cell.set_unregister_token(undefined);
+   weak_cell.set_key_list_prev(undefined);
+   weak_cell.set_key_list_next(undefined);
+diff --git a/test/cctest/test-js-weak-refs.cc b/test/cctest/test-js-weak-refs.cc
+index 8974bdf6dba65be319afbfdfc66641d27ed14a07..eb6ca2d73685b437978fac642190c8d653f4c73a 100644
+--- a/test/cctest/test-js-weak-refs.cc
++++ b/test/cctest/test-js-weak-refs.cc
+@@ -853,9 +853,7 @@ TEST(TestRemoveUnregisterToken) {
+ 
+   finalization_registry->RemoveUnregisterToken(
+       JSReceiver::cast(*token2), isolate,
+-      [undefined](WeakCell matched_cell) {
+-        matched_cell.set_unregister_token(*undefined);
+-      },
++      JSFinalizationRegistry::kKeepMatchedCellsInRegistry,
+       [](HeapObject, ObjectSlot, Object) {});
+ 
+   // Both weak_cell2a and weak_cell2b remain on the weak cell chains.
+@@ -1024,5 +1022,52 @@ TEST(UnregisterTokenHeapVerifier) {
+   EmptyMessageQueues(isolate);
+ }
+ 
++TEST(UnregisteredAndUnclearedCellHeapVerifier) {
++  if (!FLAG_incremental_marking) return;
++  ManualGCScope manual_gc_scope;
++#ifdef VERIFY_HEAP
++  FLAG_verify_heap = true;
++#endif
++
++  CcTest::InitializeVM();
++  v8::Isolate* isolate = CcTest::isolate();
++  Heap* heap = CcTest::heap();
++  v8::HandleScope outer_scope(isolate);
++
++  {
++    // Make a new FinalizationRegistry and register an object with a token.
++    v8::HandleScope scope(isolate);
++    CompileRun(
++        "var token = {}; "
++        "var registry = new FinalizationRegistry(function () {}); "
++        "registry.register({}, undefined, token);");
++  }
++
++  // Start incremental marking to activate the marking barrier.
++  heap::SimulateIncrementalMarking(heap, false);
++
++  {
++    // Make a WeakCell list with length >1, then unregister with the token to
++    // the WeakCell from the registry. The linked list manipulation keeps the
++    // unregistered WeakCell alive (i.e. not put into cleared_cells) due to the
++    // marking barrier from incremental marking. Then make the original token
++    // collectible.
++    v8::HandleScope scope(isolate);
++    CompileRun(
++        "registry.register({}); "
++        "registry.unregister(token); "
++        "token = 0;");
++  }
++
++  // Trigger GC.
++  CcTest::CollectAllGarbage();
++  CcTest::CollectAllGarbage();
++
++  // Pump message loop to run the finalizer task, then the incremental marking
++  // task. The verifier will verify that live WeakCells don't point to dead
++  // unregister tokens.
++  EmptyMessageQueues(isolate);
++}
++
+ }  // namespace internal
+ }  // namespace v8