[VM/Service] Delete all the Service ID zones created by a client when it disconnects
TEST=pkg/vm_service/test/id_zone_deletion_on_client_disconnect_with_dds_test.dart,
pkg/vm_service/test/id_zone_deletion_on_client_disconnect_without_dds_test.dart,
CI
Issue: https://github.com/dart-lang/sdk/issues/55869
CoreLibraryReviewExempt: This CL does not include any core library API
changes, only VM Service implementation changes in
sdk/lib/vmservice/client.dart and
sdk/lib/vmservice/running_isolates.dart.
Change-Id: I32a72ade89b858519c61b5124adbeafb835c3d61
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/381520
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Derek Xu <derekx@google.com>
diff --git a/pkg/dds/lib/src/client.dart b/pkg/dds/lib/src/client.dart
index 3a4acca..207f61e 100644
--- a/pkg/dds/lib/src/client.dart
+++ b/pkg/dds/lib/src/client.dart
@@ -67,7 +67,16 @@
///
/// Returned future completes when the peer is closed.
Future<void> listen() => _clientPeer.listen().then(
- (_) => dds.streamManager.clientDisconnect(this),
+ (_) async {
+ dds.streamManager.clientDisconnect(this);
+
+ for (final pair in createdServiceIdZones) {
+ await _vmServicePeer.sendRequest('deleteIdZone', {
+ 'isolateId': pair.isolateId,
+ 'idZoneId': pair.serviceIdZoneId,
+ });
+ }
+ },
);
/// Close the connection to the client.
@@ -260,6 +269,21 @@
},
);
+ _clientPeer.registerMethod('createIdZone',
+ (json_rpc.Parameters parameters) async {
+ final response = await _vmServicePeer.sendRequest(
+ parameters.method,
+ parameters.value,
+ ) as Map<String, dynamic>;
+ if (response.containsKey('id')) {
+ createdServiceIdZones.add((
+ serviceIdZoneId: response['id']!,
+ isolateId: parameters['isolateId'].asString,
+ ));
+ }
+ return response;
+ });
+
_clientPeer.registerMethod(
'getCachedCpuSamples',
dds.isolateManager.getCachedCpuSamples,
@@ -372,6 +396,12 @@
final DartDevelopmentServiceImpl dds;
final StreamChannel connection;
final Map<String, String> services = {};
+
+ /// Pairs of 1) the ID of a Service ID zone created by this client and 2) the
+ /// ID of the isolate in which that zone was created.
+ final List<({String serviceIdZoneId, String isolateId})>
+ createdServiceIdZones = [];
+
final Set<String> profilerUserTagFilters = {};
final json_rpc.Peer _vmServicePeer;
late json_rpc.Peer _clientPeer;
diff --git a/pkg/pkg.status b/pkg/pkg.status
index 3135893..5cf29a4 100644
--- a/pkg/pkg.status
+++ b/pkg/pkg.status
@@ -148,6 +148,8 @@
vm_service/test/get_stack_limit_rpc_test: SkipByDesign # Debugger is disabled in AOT mode.
vm_service/test/get_stack_rpc_test: SkipByDesign # Debugger is disabled in AOT mode.
vm_service/test/get_stack_test: SkipByDesign # Debugger is disabled in AOT mode.
+vm_service/test/id_zone_deletion_on_client_disconnect_with_dds_test: SkipByDesign # Debugger is disabled in AOT mode.
+vm_service/test/id_zone_deletion_on_client_disconnect_without_dds_test: SkipByDesign # Debugger is disabled in AOT mode.
vm_service/test/id_zones_test: SkipByDesign # Debugger is disabled in AOT mode.
vm_service/test/implicit_getter_setter_test: SkipByDesign # Debugger is disabled in AOT mode.
vm_service/test/invoke_test: SkipByDesign # Debugger is disabled in AOT mode.
diff --git a/pkg/vm_service/test/id_zone_deletion_on_client_disconnect_common.dart b/pkg/vm_service/test/id_zone_deletion_on_client_disconnect_common.dart
new file mode 100644
index 0000000..0c873f3
--- /dev/null
+++ b/pkg/vm_service/test/id_zone_deletion_on_client_disconnect_common.dart
@@ -0,0 +1,87 @@
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:developer';
+
+import 'package:test/test.dart';
+import 'package:vm_service/vm_service.dart';
+import 'package:vm_service/vm_service_io.dart';
+
+import 'common/service_test_common.dart';
+
+// AUTOGENERATED START
+//
+// Update these constants by running:
+//
+// dart pkg/vm_service/test/update_line_numbers.dart <test.dart>
+//
+const LINE_A = 25;
+// AUTOGENERATED END
+
+void testeeMain() {
+ // ignore: unused_local_variable
+ final abcString = 'abc';
+ debugger(); // LINE_A
+}
+
+final idZoneDeletionOnDisconnectTests = <IsolateTest>[
+ hasStoppedAtBreakpoint,
+ stoppedAtLine(LINE_A),
+ (VmService client1, IsolateRef isolateRef) async {
+ final isolateId = isolateRef.id!;
+
+ final idZone1 = await client1.createIdZone(
+ isolateId,
+ IdZoneBackingBufferKind.kRing,
+ IdAssignmentPolicy.kAlwaysAllocate,
+ );
+
+ // Confirm that [idZone1] can be used.
+ await client1.evaluateInFrame(
+ isolateId,
+ 0,
+ 'abcString',
+ idZoneId: idZone1.id,
+ );
+
+ final client2 = await vmServiceConnectUri(client1.wsUri!);
+ final idZone2 = await client2.createIdZone(
+ isolateId,
+ IdZoneBackingBufferKind.kRing,
+ IdAssignmentPolicy.kAlwaysAllocate,
+ );
+
+ // Confirm that [idZone2] can be used.
+ await client2.evaluateInFrame(
+ isolateId,
+ 0,
+ 'abcString',
+ idZoneId: idZone2.id,
+ );
+
+ // Disposing of [client2] should delete [idZone2];
+ await client2.dispose();
+
+ // Confirm that [idZone2] can be no longer be used.
+ try {
+ await client1.evaluateInFrame(
+ isolateId,
+ 0,
+ 'abcString',
+ idZoneId: idZone2.id,
+ );
+ fail('successfully used an ID zone that should have been deleted');
+ } on RPCError catch (e) {
+ expect(e.code, RPCErrorKind.kInvalidParams.code);
+ }
+
+ // Confirm that [idZone1] can still be used.
+ await client1.evaluateInFrame(
+ isolateId,
+ 0,
+ 'abcString',
+ idZoneId: idZone1.id,
+ );
+ },
+];
diff --git a/pkg/vm_service/test/id_zone_deletion_on_client_disconnect_with_dds_test.dart b/pkg/vm_service/test/id_zone_deletion_on_client_disconnect_with_dds_test.dart
new file mode 100644
index 0000000..c74591a
--- /dev/null
+++ b/pkg/vm_service/test/id_zone_deletion_on_client_disconnect_with_dds_test.dart
@@ -0,0 +1,13 @@
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'common/test_helper.dart';
+import 'id_zone_deletion_on_client_disconnect_common.dart';
+
+Future<void> main([args = const <String>[]]) => runIsolateTests(
+ args,
+ idZoneDeletionOnDisconnectTests,
+ 'id_zone_deletion_on_client_disconnect_without_dds_test.dart',
+ testeeConcurrent: testeeMain,
+ );
diff --git a/pkg/vm_service/test/id_zone_deletion_on_client_disconnect_without_dds_test.dart b/pkg/vm_service/test/id_zone_deletion_on_client_disconnect_without_dds_test.dart
new file mode 100644
index 0000000..06c006f
--- /dev/null
+++ b/pkg/vm_service/test/id_zone_deletion_on_client_disconnect_without_dds_test.dart
@@ -0,0 +1,14 @@
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'common/test_helper.dart';
+import 'id_zone_deletion_on_client_disconnect_common.dart';
+
+Future<void> main([args = const <String>[]]) => runIsolateTests(
+ args,
+ idZoneDeletionOnDisconnectTests,
+ 'id_zone_deletion_on_client_disconnect_without_dds_test.dart',
+ extraArgs: ['--no-dds'],
+ testeeConcurrent: testeeMain,
+ );
diff --git a/sdk/lib/vmservice/client.dart b/sdk/lib/vmservice/client.dart
index 7ce9c51..3726fb9 100644
--- a/sdk/lib/vmservice/client.dart
+++ b/sdk/lib/vmservice/client.dart
@@ -23,6 +23,11 @@
/// The set of streams the client is subscribed to.
final streams = <String>{};
+ /// Pairs of 1) the ID of a Service ID zone created by this client and 2) the
+ /// ID of the isolate in which that zone was created.
+ final List<({String serviceIdZoneId, String isolateId})>
+ createdServiceIdZones = [];
+
/// The set of user tags that the client wants to receive CPU samples for.
final profilerUserTagFilters = <String>{};
diff --git a/sdk/lib/vmservice/vmservice.dart b/sdk/lib/vmservice/vmservice.dart
index 9fc218f..65b37db 100644
--- a/sdk/lib/vmservice/vmservice.dart
+++ b/sdk/lib/vmservice/vmservice.dart
@@ -285,6 +285,17 @@
_vmCancelStream(streamId);
}
}
+ for (final pair in client.createdServiceIdZones) {
+ runningIsolates.routeRequest(
+ this,
+ Message._fromJsonRpcRequest(client, {
+ 'method': 'deleteIdZone',
+ 'params': {
+ 'isolateId': pair.isolateId,
+ 'idZoneId': pair.serviceIdZoneId,
+ }
+ }));
+ }
_cleanupUnusedUserTagSubscriptions();
for (final service in client.services.keys) {
_eventMessageHandler(
@@ -725,6 +736,20 @@
return encodeSuccess(message);
}
+ void _recordInformationAboutCreatedServiceIdZone(
+ Client client,
+ Map<String, dynamic> decodedResponse,
+ String isolateId,
+ ) {
+ if (decodedResponse.containsKey('result')) {
+ client.createdServiceIdZones.add((
+ serviceIdZoneId:
+ (decodedResponse['result'] as Map<String, dynamic>)['id']!,
+ isolateId: isolateId,
+ ));
+ }
+ }
+
Future<Response?> routeRequest(VMService _, Message message) async {
final response = await _routeRequestImpl(message);
if (response == null) {
@@ -769,7 +794,16 @@
return await _handleService(message);
}
if (message.params['isolateId'] != null) {
- return await runningIsolates.routeRequest(this, message);
+ final Response response =
+ await runningIsolates.routeRequest(this, message);
+ if (message.method == 'createIdZone') {
+ _recordInformationAboutCreatedServiceIdZone(
+ message.client!,
+ response.decodeJson(),
+ message.params['isolateId'],
+ );
+ }
+ return response;
}
return await message.sendToVM();
} catch (e, st) {
