Commit 705527f8 authored by Shu-yu Guo's avatar Shu-yu Guo Committed by V8 LUCI CQ

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

This reverts commit 360c7afc.

Reason for revert: TSAN https://ci.chromium.org/ui/p/v8/builders/ci/V8%20Linux64%20TSAN%20-%20stress-incremental-marking/3437/overview

Original change's description:
> [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/+/2923294
> Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
> Commit-Queue: Shu-yu Guo <syg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#74890}

Bug: chromium:1213770
Change-Id: I9655db1a20d983c187779199e9009f6aeb5b46df
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2930553
Auto-Submit: Shu-yu Guo <syg@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/heads/master@{#74893}
parent ed7e4554
......@@ -2578,6 +2578,11 @@ 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);
......
......@@ -6844,7 +6844,6 @@ 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
......@@ -6868,7 +6867,8 @@ 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(undefined);
next.set_key_list_prev(ReadOnlyRoots(isolate).undefined_value());
weak_cell.set_key_list_next(ReadOnlyRoots(isolate).undefined_value());
key_map.ValueAtPut(entry, next);
}
} else {
......@@ -6880,12 +6880,6 @@ 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
......
......@@ -979,17 +979,15 @@ TEST(UnregisterTokenHeapVerifier) {
v8::HandleScope outer_scope(isolate);
{
// Make a new FinalizationRegistry and register two objects with the same
// unregister token that's unreachable after the IIFE returns.
// Make a new FinalizationRegistry and register an object with an unregister
// token that's unreachable after the IIFE returns.
v8::HandleScope scope(isolate);
CompileRun(
"var token = {}; "
"var registry = new FinalizationRegistry(function () {}); "
"(function () { "
" let o1 = {}; "
" let o2 = {}; "
" registry.register(o1, {}, token); "
" registry.register(o2, {}, token); "
" let o = {}; "
" registry.register(o, {}, token); "
"})();");
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment