Skip to content

Commit

Permalink
[symbol-as-weakmap-key] Fix DCHECKs when clearing JS weakrefs
Browse files Browse the repository at this point in the history
Bug: chromium:1372500, v8:12947
Fixed: chromium:1372500
Change-Id: Id6330de5886e4ea72544b307c358e2190ea47d9c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3942586
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#83632}
  • Loading branch information
syg authored and V8 LUCI CQ committed Oct 11, 2022
1 parent e977516 commit 1fada6b
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/heap/mark-compact.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3479,7 +3479,7 @@ void MarkCompactCollector::ClearJSWeakRefs() {
};
HeapObject target = HeapObject::cast(weak_cell.target());
if (!non_atomic_marking_state()->IsBlackOrGrey(target)) {
DCHECK(!target.IsUndefined());
DCHECK(target.CanBeHeldWeakly());
// The value of the WeakCell is dead.
JSFinalizationRegistry finalization_registry =
JSFinalizationRegistry::cast(weak_cell.finalization_registry());
Expand All @@ -3501,6 +3501,7 @@ void MarkCompactCollector::ClearJSWeakRefs() {

HeapObject unregister_token = weak_cell.unregister_token();
if (!non_atomic_marking_state()->IsBlackOrGrey(unregister_token)) {
DCHECK(unregister_token.CanBeHeldWeakly());
// The unregister token is dead. Remove any corresponding entries in the
// key map. Multiple WeakCell with the same token will have all their
// unregister_token field set to undefined when processing the first
Expand All @@ -3509,7 +3510,7 @@ void MarkCompactCollector::ClearJSWeakRefs() {
JSFinalizationRegistry finalization_registry =
JSFinalizationRegistry::cast(weak_cell.finalization_registry());
finalization_registry.RemoveUnregisterToken(
JSReceiver::cast(unregister_token), isolate(),
unregister_token, isolate(),
JSFinalizationRegistry::kKeepMatchedCellsInRegistry,
gc_notify_updated_slot);
} else {
Expand Down
14 changes: 14 additions & 0 deletions test/mjsunit/harmony/regress/regress-crbug-1372500.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright 2022 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: --harmony-symbol-as-weakmap-key --expose-gc

// Register an object in a FinalizationRegistry with a Symbol as the unregister
// token.
let fr = new FinalizationRegistry(function () {});
(function register() {
fr.register({}, "holdings", Symbol('unregisterToken'));
})();
// The unregister token should be dead, trigger its collection.
gc();

0 comments on commit 1fada6b

Please sign in to comment.