From 4d58f8acc5441313063b8b6be3547e93ace20fdd Mon Sep 17 00:00:00 2001 From: Seth Brenith Date: Tue, 30 Nov 2021 09:58:39 -0800 Subject: [PATCH] Make JSFinalizationRegistry::next_dirty weak Currently, JSFinalizationRegistry has a BodyDescriptor that iterates next_dirty as a custom weak field, and it has a WeakListVisitor that cleans up any items from the list that should be removed. However, none of that code is used, because JSFinalizationRegistry objects are created with visitor ID kVisitJSObjectFast. This change gives them a custom visitor ID so that next_dirty can be treated as weak. Bug: v8:12430 Change-Id: I31c1935257ad508b13a3e684662d2ca406d8ed19 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3307096 Commit-Queue: Seth Brenith Reviewed-by: Yang Guo Reviewed-by: Michael Lippautz Cr-Commit-Position: refs/heads/main@{#78167} --- src/heap/concurrent-marking.cc | 12 +++++-- src/heap/memory-measurement-inl.h | 1 + src/heap/objects-visiting.h | 1 + src/objects/map.cc | 4 ++- src/objects/map.h | 1 + src/profiler/heap-snapshot-generator.cc | 3 ++ ...nregistry-independent-lifetime-multiple.js | 32 +++++++++++++++++++ 7 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 test/mjsunit/harmony/weakrefs/finalizationregistry-independent-lifetime-multiple.js diff --git a/src/heap/concurrent-marking.cc b/src/heap/concurrent-marking.cc index 3bd3e4730d..54d73799b7 100644 --- a/src/heap/concurrent-marking.cc +++ b/src/heap/concurrent-marking.cc @@ -127,6 +127,10 @@ class ConcurrentMarkingVisitor final return VisitJSObjectSubclass(map, object); } + int VisitJSFinalizationRegistry(Map map, JSFinalizationRegistry object) { + return VisitJSObjectSubclass(map, object); + } + int VisitConsString(Map map, ConsString object) { return VisitFullyWithSnapshot(map, object); } @@ -210,19 +214,21 @@ class ConcurrentMarkingVisitor final void VisitCodeTarget(Code host, RelocInfo* rinfo) final { // This should never happen, because snapshotting is performed only on - // JSObjects (and derived classes). + // some String subclasses. UNREACHABLE(); } void VisitEmbeddedPointer(Code host, RelocInfo* rinfo) final { // This should never happen, because snapshotting is performed only on - // JSObjects (and derived classes). + // some String subclasses. UNREACHABLE(); } void VisitCustomWeakPointers(HeapObject host, ObjectSlot start, ObjectSlot end) override { - DCHECK(host.IsWeakCell() || host.IsJSWeakRef()); + // This should never happen, because snapshotting is performed only on + // some String subclasses. + UNREACHABLE(); } private: diff --git a/src/heap/memory-measurement-inl.h b/src/heap/memory-measurement-inl.h index f6c75b6ca6..6924bbf1b1 100644 --- a/src/heap/memory-measurement-inl.h +++ b/src/heap/memory-measurement-inl.h @@ -29,6 +29,7 @@ bool NativeContextInferrer::Infer(Isolate* isolate, Map map, HeapObject object, native_context); case kVisitJSApiObject: case kVisitJSArrayBuffer: + case kVisitJSFinalizationRegistry: case kVisitJSObject: case kVisitJSObjectFast: case kVisitJSTypedArray: diff --git a/src/heap/objects-visiting.h b/src/heap/objects-visiting.h index 73b566279e..858e279ec4 100644 --- a/src/heap/objects-visiting.h +++ b/src/heap/objects-visiting.h @@ -30,6 +30,7 @@ namespace internal { V(FixedDoubleArray) \ V(JSArrayBuffer) \ V(JSDataView) \ + V(JSFinalizationRegistry) \ V(JSFunction) \ V(JSObject) \ V(JSTypedArray) \ diff --git a/src/objects/map.cc b/src/objects/map.cc index d0f74781ab..776d66b42d 100644 --- a/src/objects/map.cc +++ b/src/objects/map.cc @@ -249,7 +249,6 @@ VisitorId Map::GetVisitorId(Map map) { case JS_CONTEXT_EXTENSION_OBJECT_TYPE: case JS_DATE_TYPE: case JS_ERROR_TYPE: - case JS_FINALIZATION_REGISTRY_TYPE: case JS_GENERATOR_OBJECT_TYPE: case JS_ITERATOR_PROTOTYPE_TYPE: case JS_MAP_ITERATOR_PROTOTYPE_TYPE: @@ -324,6 +323,9 @@ VisitorId Map::GetVisitorId(Map map) { case WEAK_CELL_TYPE: return kVisitWeakCell; + case JS_FINALIZATION_REGISTRY_TYPE: + return kVisitJSFinalizationRegistry; + case FILLER_TYPE: case FOREIGN_TYPE: case HEAP_NUMBER_TYPE: diff --git a/src/objects/map.h b/src/objects/map.h index f0c7e627f2..f357622ed8 100644 --- a/src/objects/map.h +++ b/src/objects/map.h @@ -46,6 +46,7 @@ enum InstanceType : uint16_t; V(JSApiObject) \ V(JSArrayBuffer) \ V(JSDataView) \ + V(JSFinalizationRegistry) \ V(JSFunction) \ V(JSObject) \ V(JSObjectFast) \ diff --git a/src/profiler/heap-snapshot-generator.cc b/src/profiler/heap-snapshot-generator.cc index bf8e23bc45..e911636677 100644 --- a/src/profiler/heap-snapshot-generator.cc +++ b/src/profiler/heap-snapshot-generator.cc @@ -1748,6 +1748,9 @@ bool V8HeapExplorer::IsEssentialHiddenReference(Object parent, if (parent.IsContext() && field_offset == Context::OffsetOfElementAt(Context::NEXT_CONTEXT_LINK)) return false; + if (parent.IsJSFinalizationRegistry() && + field_offset == JSFinalizationRegistry::kNextDirtyOffset) + return false; return true; } diff --git a/test/mjsunit/harmony/weakrefs/finalizationregistry-independent-lifetime-multiple.js b/test/mjsunit/harmony/weakrefs/finalizationregistry-independent-lifetime-multiple.js new file mode 100644 index 0000000000..4eb54166ea --- /dev/null +++ b/test/mjsunit/harmony/weakrefs/finalizationregistry-independent-lifetime-multiple.js @@ -0,0 +1,32 @@ +// Copyright 2021 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: --expose-gc --noincremental-marking --no-concurrent-inlining + +let cleanup_called = false; +function cleanup(holdings) { + cleanup_called = true; +}; +let cleanup_called_2 = false; +function cleanup2(holdings) { + cleanup_called_2 = true; +}; +let fg = new FinalizationRegistry(cleanup); +(function() { + let fg2 = new FinalizationRegistry(cleanup2); + (function() { + fg.register({}, {}); + fg2.register({}, {}); + })(); + // Schedule fg and fg2 for cleanup. + gc(); +})(); + +// Collect fg2, but fg is still alive. +gc(); + +setTimeout(function() { + assertTrue(cleanup_called); + assertFalse(cleanup_called_2); +}, 0);