[vm, service] Treat the object id ring as strong during major GC.

The id ring was treated as a strong root during minor GC and a weak root during major GC. Before mark-through-new-space, new-space objects could not be collected during a major GC, so in practice the policy was that the id ring was strong for all new-space objects. After mark-through-new-space, new-space object can be collected during a major GC, so service ids could expire very quickly, especially for object just created using the service protocol and not otherwise reachable from the Dart program. Given that service id zones have still not been implemented, this makes it impossible to reliably interact with such an object.

TEST=ci
Bug: https://github.com/flutter/flutter/issues/148704
Change-Id: I0f15c00414f996fad49bcb137c7f1c15bb4955c4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/367440
Reviewed-by: Derek Xu <derekx@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
This commit is contained in:
Ryan Macnak 2024-05-21 21:29:49 +00:00 committed by Commit Queue
parent e5e2c925c0
commit ffbbdb5a10
3 changed files with 16 additions and 47 deletions

View file

@ -699,7 +699,8 @@ void GCMarker::Epilogue() {}
enum RootSlices {
kIsolate = 0,
kNumFixedRootSlices = 1,
kObjectIdRing = 1,
kNumFixedRootSlices = 2,
};
void GCMarker::ResetSlices() {
@ -727,6 +728,12 @@ void GCMarker::IterateRoots(ObjectPointerVisitor* visitor) {
visitor, ValidationPolicy::kDontValidateFrames);
break;
}
case kObjectIdRing: {
TIMELINE_FUNCTION_GC_DURATION(Thread::Current(),
"ProcessObjectIdTable");
isolate_group_->VisitObjectIdRingPointers(visitor);
break;
}
}
MonitorLocker ml(&root_slices_monitor_);
@ -740,7 +747,6 @@ void GCMarker::IterateRoots(ObjectPointerVisitor* visitor) {
enum WeakSlices {
kWeakHandles = 0,
kWeakTables,
kObjectIdRing,
kRememberedSet,
kNumWeakSlices,
};
@ -759,9 +765,6 @@ void GCMarker::IterateWeakRoots(Thread* thread) {
case kWeakTables:
ProcessWeakTables(thread);
break;
case kObjectIdRing:
ProcessObjectIdTable(thread);
break;
case kRememberedSet:
ProcessRememberedSet(thread);
break;
@ -851,39 +854,6 @@ void GCMarker::ProcessRememberedSet(Thread* thread) {
store_buffer->PushBlock(writing, StoreBuffer::kIgnoreThreshold);
}
class ObjectIdRingClearPointerVisitor : public ObjectPointerVisitor {
public:
explicit ObjectIdRingClearPointerVisitor(IsolateGroup* isolate_group)
: ObjectPointerVisitor(isolate_group) {}
void VisitPointers(ObjectPtr* first, ObjectPtr* last) override {
for (ObjectPtr* current = first; current <= last; current++) {
ObjectPtr obj = *current;
ASSERT(obj->IsHeapObject());
if (!obj->untag()->IsMarked()) {
// Object has become garbage. Replace it will null.
*current = Object::null();
}
}
}
#if defined(DART_COMPRESSED_POINTERS)
void VisitCompressedPointers(uword heap_base,
CompressedObjectPtr* first,
CompressedObjectPtr* last) override {
UNREACHABLE(); // ObjectIdRing is not compressed.
}
#endif
};
void GCMarker::ProcessObjectIdTable(Thread* thread) {
#ifndef PRODUCT
TIMELINE_FUNCTION_GC_DURATION(thread, "ProcessObjectIdTable");
ObjectIdRingClearPointerVisitor visitor(isolate_group_);
isolate_group_->VisitObjectIdRingPointers(&visitor);
#endif // !PRODUCT
}
class ParallelMarkTask : public ThreadPool::Task {
public:
ParallelMarkTask(GCMarker* marker,

View file

@ -62,7 +62,6 @@ class GCMarker {
void ProcessWeakHandles(Thread* thread);
void ProcessWeakTables(Thread* thread);
void ProcessRememberedSet(Thread* thread);
void ProcessObjectIdTable(Thread* thread);
// Called by anyone: finalize and accumulate stats from 'visitor'.
template <class MarkingVisitorType>

View file

@ -195,7 +195,7 @@ TEST_CASE(ObjectIdRingScavengeMoveTest) {
EXPECT_EQ(3, list_length);
}
// Test that the ring table is updated with nulls when the old GC collects.
// Test that the ring table is updated when major GC runs.
ISOLATE_UNIT_TEST_CASE(ObjectIdRingOldGCTest) {
Isolate* isolate = thread->isolate();
ObjectIdRing* ring = isolate->EnsureObjectIdRing();
@ -229,16 +229,16 @@ ISOLATE_UNIT_TEST_CASE(ObjectIdRingOldGCTest) {
UntaggedObject::ToAddr(raw_obj2));
// Exit scope. Freeing String handle.
}
// Force a GC. No reference exist to the old string anymore. It should be
// collected and the object id ring will now return the null object for
// those ids.
// Force a GC. No other reference to the old string exists, but the service id
// should keep it alive.
GCTestHelper::CollectOldSpace();
ObjectPtr raw_object_moved1 = ring->GetObjectForId(raw_obj_id1, &kind);
EXPECT_EQ(ObjectIdRing::kCollected, kind);
EXPECT_EQ(Object::null(), raw_object_moved1);
EXPECT_EQ(ObjectIdRing::kValid, kind);
EXPECT(raw_object_moved1->IsOneByteString());
ObjectPtr raw_object_moved2 = ring->GetObjectForId(raw_obj_id2, &kind);
EXPECT_EQ(ObjectIdRing::kCollected, kind);
EXPECT_EQ(Object::null(), raw_object_moved2);
EXPECT_EQ(ObjectIdRing::kValid, kind);
EXPECT(raw_object_moved2->IsOneByteString());
EXPECT_EQ(raw_object_moved1, raw_object_moved2);
}
// Test that the ring table correctly reports an entry as expired when it is