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

* chore: [17-x-y] cherry-pick 44c4e56fea2c from v8

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
This commit is contained in:
Jeremy Rose
2022-06-23 08:17:03 -07:00
committed by GitHub
parent b36b2e3781
commit f0f2c861bb
2 changed files with 186 additions and 0 deletions

View File

@@ -14,3 +14,4 @@ cherry-pick-e42dbcdedb7a.patch
cherry-pick-b2d3ef69ef99.patch
cherry-pick-2004594a46c8.patch
cherry-pick-723ed8a9cfff.patch
cherry-pick-44c4e56fea2c.patch

View File

@@ -0,0 +1,185 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shu-yu Guo <syg@chromium.org>
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 <dinfuehr@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
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 2977b4219d68c0e31bfa4224db16fc1ef31112ce..8d27fe3d35311d2b700d2cb1bceff47ab2f3640d 100644
--- a/src/heap/mark-compact.cc
+++ b/src/heap/mark-compact.cc
@@ -2905,14 +2905,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 2ddcedaa5815d6905b587e733c7c276aea234e08..24d474332bec9a83391d44b53aa955f8e0299b8b 100644
--- a/src/objects/objects.cc
+++ b/src/objects/objects.cc
@@ -6882,7 +6882,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