From daa8cbb29e5cb4458704edc1ada2b3261d8e3a10 Mon Sep 17 00:00:00 2001 From: Derek Xu Date: Wed, 28 Aug 2024 16:41:50 +0000 Subject: [PATCH] [VM/Service] Add private _invalidateIdZone RPC TEST=pkg/vm_service/test/id_zones_test.dart, CI Issue: https://github.com/dart-lang/sdk/issues/55869 Change-Id: I02fcb2502b698066885b3f090435e43a34ed6fcd Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/379820 Reviewed-by: Ben Konyi --- pkg/vm_service/test/id_zones_test.dart | 60 ++++++++++++++++++++++++++ runtime/vm/object_id_ring.cc | 19 +++++--- runtime/vm/object_id_ring.h | 3 ++ runtime/vm/service.cc | 23 ++++++++++ runtime/vm/service.h | 3 ++ 5 files changed, 102 insertions(+), 6 deletions(-) diff --git a/pkg/vm_service/test/id_zones_test.dart b/pkg/vm_service/test/id_zones_test.dart index 5d9fa387b9d..708949175bc 100644 --- a/pkg/vm_service/test/id_zones_test.dart +++ b/pkg/vm_service/test/id_zones_test.dart @@ -85,6 +85,36 @@ final tests = [ final cInstance2 = await service.getObject(isolateId, cObjectId2); expect(cInstance2.type, 'Instance'); + + await service.callMethod( + '_invalidateIdZone', + isolateId: isolateId, + args: { + '_idZoneId': idZone1['id'], + }, + ); + + try { + await service.getObject(isolateId, cObjectId1); + fail('successfully retrieved object using expired ID'); + } on SentinelException catch (e) { + expect(e.sentinel.kind, startsWith('Expired')); + expect(e.sentinel.valueAsString, equals('')); + } + + // Ensure that the zone can be reused after it was invalidated. + final cInstanceRef3 = (await service.callMethod( + 'evaluateInFrame', + isolateId: isolateId, + args: { + 'frameIndex': 0, + 'expression': 'c', + '_idZoneId': idZone1['id'], + }, + )) + .json!; + expect(cInstanceRef3['type'], '@Instance'); + expect(cInstanceRef3['id'], 'objects/0/1'); }, // Test the behaviour of an ID Zone with a `backingBufferKind` of `Ring`, an @@ -202,6 +232,36 @@ final tests = [ final cInstance2 = await service.getObject(isolateId, cObjectId2); expect(cInstance2.type, 'Instance'); + + await service.callMethod( + '_invalidateIdZone', + isolateId: isolateId, + args: { + '_idZoneId': idZone3['id'], + }, + ); + + try { + await service.getObject(isolateId, cObjectId1); + fail('successfully retrieved object using expired ID'); + } on SentinelException catch (e) { + expect(e.sentinel.kind, startsWith('Expired')); + expect(e.sentinel.valueAsString, equals('')); + } + + // Ensure that the zone can be reused after it was invalidated. + final cInstanceRef3 = (await service.callMethod( + 'evaluateInFrame', + isolateId: isolateId, + args: { + 'frameIndex': 0, + 'expression': 'c', + '_idZoneId': idZone3['id'], + }, + )) + .json!; + expect(cInstanceRef3['type'], '@Instance'); + expect(cInstanceRef3['id'], 'objects/0/3'); }, resumeIsolate, ]; diff --git a/runtime/vm/object_id_ring.cc b/runtime/vm/object_id_ring.cc index f1d0a4f9bb0..5f08aa4693b 100644 --- a/runtime/vm/object_id_ring.cc +++ b/runtime/vm/object_id_ring.cc @@ -18,9 +18,17 @@ ObjectIdRing::~ObjectIdRing() { table_ = nullptr; } +void ObjectIdRing::Invalidate() { + serial_num_ = 0; + wrapped_ = false; + for (int32_t i = 0; i < capacity_; i++) { + table_[i] = Object::null(); + } +} + int32_t ObjectIdRing::GetIdForObject(ObjectPtr object, IdPolicy policy) { - // We do not allow inserting null because null is how we detect as entry was - // reclaimed by the GC. + // We do not allow inserting |Object::null()| because we use it as a + // placeholder for unpopulated slots in |table_|. ASSERT(object != Object::null()); if (policy == kAllocateId) { return AllocateNewId(object); @@ -51,10 +59,9 @@ ObjectPtr ObjectIdRing::GetObjectForId(int32_t id, LookupResult* kind) { } ASSERT(index >= 0); ASSERT(index < capacity_); - if (table_[index] == Object::null()) { - *kind = kCollected; - return Object::null(); - } + // The `index == kInvalidId` check above should make it impossible for + // `table_[index]` to be |Object::null()|. + ASSERT(table_[index] != Object::null()); *kind = kValid; ASSERT(IdOfIndex(index) == id); return table_[index]; diff --git a/runtime/vm/object_id_ring.h b/runtime/vm/object_id_ring.h index 4291503e1b5..3f7c8502375 100644 --- a/runtime/vm/object_id_ring.h +++ b/runtime/vm/object_id_ring.h @@ -44,6 +44,9 @@ class ObjectIdRing { explicit ObjectIdRing(int32_t capacity); ~ObjectIdRing(); + // Invalidate all the Service IDs currently living in this ring. + void Invalidate(); + // Adds the argument to the ring and returns its id. Note we do not allow // adding Object::null(). int32_t GetIdForObject(ObjectPtr raw_obj, IdPolicy policy = kAllocateId); diff --git a/runtime/vm/service.cc b/runtime/vm/service.cc index ad1d53a8b1f..174eacb4b9e 100644 --- a/runtime/vm/service.cc +++ b/runtime/vm/service.cc @@ -395,6 +395,10 @@ char* RingServiceIdZone::GetServiceId(const Object& obj) { id()); } +void RingServiceIdZone::Invalidate() { + ring_.Invalidate(); +} + void RingServiceIdZone::VisitPointers(ObjectPointerVisitor& visitor) const { ring_.VisitPointers(&visitor); } @@ -2682,6 +2686,23 @@ static void GetReachableSize(Thread* thread, JSONStream* js) { result.PrintJSON(js, true); } +static const MethodParameter* const invalidate_id_zone_params[] = { + RUNNABLE_ISOLATE_PARAMETER, + nullptr, +}; + +static void InvalidateIdZone(Thread* thread, JSONStream* js) { + ASSERT(thread != nullptr); + ASSERT(js != nullptr); + + Isolate* isolate = thread->isolate(); + ASSERT(isolate != nullptr); + + js->id_zone().Invalidate(); + + PrintSuccess(js); +} + static const MethodParameter* const invoke_params[] = { RUNNABLE_ISOLATE_PARAMETER, nullptr, @@ -6087,6 +6108,8 @@ static const ServiceMethodDescriptor service_methods_[] = { get_vm_timeline_flags_params }, { "getVMTimelineMicros", GetVMTimelineMicros, get_vm_timeline_micros_params }, + { "_invalidateIdZone", InvalidateIdZone, + invalidate_id_zone_params }, { "invoke", Invoke, invoke_params }, { "kill", Kill, kill_params }, { "pause", Pause, diff --git a/runtime/vm/service.h b/runtime/vm/service.h index d400601a4a3..1eddf42db17 100644 --- a/runtime/vm/service.h +++ b/runtime/vm/service.h @@ -48,6 +48,8 @@ class ServiceIdZone { ObjectIdRing::LookupResult* kind) = 0; // Returned string will be zone allocated. virtual char* GetServiceId(const Object& obj) = 0; + // Invalidate all the Service IDs currently living in this zone. + virtual void Invalidate() = 0; virtual void VisitPointers(ObjectPointerVisitor& visitor) const = 0; virtual void PrintJSON(JSONStream& js) const = 0; @@ -77,6 +79,7 @@ class RingServiceIdZone final : public ServiceIdZone { ObjectPtr GetObjectForId(int32_t id, ObjectIdRing::LookupResult* kind) final; // Returned string will be zone allocated. char* GetServiceId(const Object& obj) final; + void Invalidate() final; void VisitPointers(ObjectPointerVisitor& visitor) const final; void PrintJSON(JSONStream& js) const final;