diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md index ca931328f58..8125b35f331 100644 --- a/pkg/dds/CHANGELOG.md +++ b/pkg/dds/CHANGELOG.md @@ -1,3 +1,8 @@ +# 4.0.0 +- Updated DDS protocol to version 2.0. +- Added `readyToResume` and `requireUserPermissionToResume` RPCs. +- **Breaking change:** `resume` is now treated as a user-initiated resume request and force resumes paused isolates, regardless of required resume approvals. Tooling relying on resume permissions should use the `readyToResume` RPC to indicate to DDS that they are ready to resume. + # 3.4.0 - Start the Dart Tooling Daemon from the DevTools server when a connection is not passed to the server on start. diff --git a/pkg/dds/dds_protocol.md b/pkg/dds/dds_protocol.md index 49f84123566..fba9db4d945 100644 --- a/pkg/dds/dds_protocol.md +++ b/pkg/dds/dds_protocol.md @@ -1,6 +1,6 @@ -# Dart Development Service Protocol 1.5 +# Dart Development Service Protocol 2.0 -This document describes _version 1.5_ of the Dart Development Service Protocol. +This document describes _version 2.0_ of the Dart Development Service Protocol. This protocol is an extension of the Dart VM Service Protocol and implements it in it's entirety. For details on the VM Service Protocol, see the [Dart VM Service Protocol Specification][service-protocol]. @@ -172,6 +172,26 @@ void postEvent(String stream, String eventKind, Map eventData) ``` The _postEvent_ RPC is used to send events to custom Event streams. +### readyToResume + +``` +Success readyToResume(string isolateId) +``` + +The _readyToResume_ RPC indicates to DDS that the current client is ready +to resume the isolate. + +If the current client requires that approval be given before resuming an +isolate, this method will: + +- Update the approval state for the isolate. +- Resume the isolate if approval has been given by all clients which + require approval. + +Returns a collected sentinel if the isolate no longer exists. + +See [Success](#success). + ### requirePermissionToResume ``` @@ -184,6 +204,8 @@ The _requirePermissionToResume_ RPC is used to change the pause/resume behavior of isolates by providing a way for the VM service to wait for approval to resume from some set of clients. This is useful for clients which want to perform some operation on an isolate after a pause without it being resumed by another client. +These clients should invoke [readyToResume](#readyToResume) instead of [resume](resume) +to indicate to DDS that they have finished their work and the isolate can be resumed. If the _onPauseStart_ parameter is `true`, isolates will not resume after pausing on start until the client sends a `resume` request and all other clients which @@ -206,6 +228,30 @@ need to provide resume approval for this pause type have done so. already given approval. In the case that no other client requires resume approval for the current pause event, the isolate will be resumed if at least one other client has attempted to [resume](resume) the isolate. +- Resume permission behavior can be bypassed using the [VmService.resume] + RPC, which is treated as a user-initiated resume that force resumes + the isolate. Tooling relying on resume permissions should use + [readyToResume](#readyToResume) instead of [resume](resume) to avoid force + resuming the isolate. + +See [Success](#success). + +### requireUserPermissionToResume + +``` +Success requireUserPermissionToResume(bool onPauseStart [optional], + bool onPauseExit [optional]) +``` + +The _requireUserPermissionToResume_ RPC notifies DDS if it should wait +for a [resume](resume) request to resume isolates paused on start or +exit. + +This RPC should only be invoked by tooling which launched the target Dart +process and knows if the user indicated they wanted isolates paused on +start or exit. + +See [Success](#success). ### setClientName diff --git a/pkg/dds/lib/dds.dart b/pkg/dds/lib/dds.dart index cbf2c1bd494..ab4de82862f 100644 --- a/pkg/dds/lib/dds.dart +++ b/pkg/dds/lib/dds.dart @@ -166,7 +166,7 @@ abstract class DartDevelopmentService { /// The version of the DDS protocol supported by this [DartDevelopmentService] /// instance. - static const String protocolVersion = '1.6'; + static const String protocolVersion = '2.0'; } class DartDevelopmentServiceException implements Exception { diff --git a/pkg/dds/lib/src/client.dart b/pkg/dds/lib/src/client.dart index a8d7b3baf24..3a4acca3519 100644 --- a/pkg/dds/lib/src/client.dart +++ b/pkg/dds/lib/src/client.dart @@ -179,6 +179,19 @@ class DartDevelopmentServiceClient { (parameters) => dds.isolateManager.resumeIsolate(this, parameters), ); + _clientPeer.registerMethod( + 'readyToResume', + (parameters) => dds.isolateManager.readyToResume(this, parameters), + ); + + _clientPeer.registerMethod( + 'requireUserPermissionToResume', + (parameters) => dds.isolateManager.requireUserPermissionToResume( + this, + parameters, + ), + ); + _clientPeer.registerMethod('getStreamHistory', (parameters) { final stream = parameters['stream'].asString; final events = dds.streamManager.getStreamHistory(stream); diff --git a/pkg/dds/lib/src/client_manager.dart b/pkg/dds/lib/src/client_manager.dart index 2d42b7f96da..1622e31c034 100644 --- a/pkg/dds/lib/src/client_manager.dart +++ b/pkg/dds/lib/src/client_manager.dart @@ -81,10 +81,10 @@ class ClientManager { } /// Require permission from this client before resuming an isolate. - Map requirePermissionToResume( + Future> requirePermissionToResume( DartDevelopmentServiceClient client, json_rpc.Parameters parameters, - ) { + ) async { int pauseTypeMask = 0; if (parameters['onPauseStart'].asBoolOr(false)) { pauseTypeMask |= PauseTypeMasks.pauseOnStartMask; @@ -95,8 +95,12 @@ class ClientManager { if (parameters['onPauseExit'].asBoolOr(false)) { pauseTypeMask |= PauseTypeMasks.pauseOnExitMask; } - clientResumePermissions[client.name!]!.permissionsMask = pauseTypeMask; + + // Check to see if any isolates should resume as a result of the + // resume permissions being updated. + await dds.isolateManager.maybeResumeIsolates(); + return RPCResponses.success; } diff --git a/pkg/dds/lib/src/isolate_manager.dart b/pkg/dds/lib/src/isolate_manager.dart index ddee5f959a8..e5f8363b8b1 100644 --- a/pkg/dds/lib/src/isolate_manager.dart +++ b/pkg/dds/lib/src/isolate_manager.dart @@ -90,6 +90,16 @@ class RunningIsolate { // Mark approval by the client. _resumeApprovalsByName.add(resumingClient.name); } + + // If the user is required to resume the isolate, we won't resume until a + // `resume` request is received or `setRequireUserPermissionToResume` is + // invoked and removes the requirement for this pause type. + final userPermissionMask = + isolateManager._requireUserPermissionToResumeMask; + if (userPermissionMask & _isolateStateMask != 0) { + return false; + } + final requiredClientApprovals = {}; final permissions = isolateManager.dds.clientManager.clientResumePermissions; @@ -293,17 +303,40 @@ class IsolateManager { isolates.remove(id); } - /// Handles `resume` RPC requests. If the client requires that approval be - /// given before resuming an isolate, this method will: + /// Handles `resume` RPC requests. /// - /// - Update the approval state for the isolate. - /// - Resume the isolate if approval has been given by all clients which - /// require approval. + /// Invocations of `resume` are treated as user initiated and will bypass any + /// resume permissions set by tooling, force resuming the isolate and clear + /// any resume approvals. /// /// Returns a collected sentinel if the isolate no longer exists. Future> resumeIsolate( DartDevelopmentServiceClient client, json_rpc.Parameters parameters, + ) async { + return await _mutex.runGuarded( + () async { + final isolateId = parameters['isolateId'].asString; + final isolate = isolates[isolateId]; + if (isolate == null) { + return RPCResponses.collectedSentinel; + } + return await _resumeCommon(isolate, parameters); + }, + ); + } + + /// Handles `readyToResume` RPC requests. If the client requires + /// that approval be given before resuming an isolate, this method will: + /// + /// - Update the approval state for the isolate. + /// - Resume the isolate if approval has been given by all clients which + /// require approval. + /// + /// Returns a collected sentinel if the isolate no longer exists. + Future> readyToResume( + DartDevelopmentServiceClient client, + json_rpc.Parameters parameters, ) async { return await _mutex.runGuarded( () async { @@ -313,14 +346,75 @@ class IsolateManager { return RPCResponses.collectedSentinel; } if (isolate.shouldResume(resumingClient: client)) { - isolate.clearResumeApprovals(); - return await _sendResumeRequest(isolateId, parameters); + return await _resumeCommon(isolate, parameters); } return RPCResponses.success; }, ); } + Future maybeResumeIsolates() async { + await _mutex.runGuarded(() async { + for (final isolate in isolates.values) { + if (isolate.shouldResume()) { + await _resumeCommon(isolate, json_rpc.Parameters('', {})); + } + } + }); + } + + Future> _resumeCommon( + RunningIsolate isolate, + json_rpc.Parameters parameters, + ) async { + isolate.clearResumeApprovals(); + return await _sendResumeRequest(isolate.id, parameters); + } + + /// Handles `requireUserPermissionToResume` requests. + /// + /// Notifies DDS if it should wait for a `resume` request to resume isolates + /// paused on start or exit. + /// + /// This RPC should only be invoked by tooling which launched the target Dart + /// process and knows if the user indicated they wanted isolates paused on + /// start or exit. + Future> requireUserPermissionToResume( + DartDevelopmentServiceClient client, + json_rpc.Parameters parameters, + ) async { + int pauseTypeMask = 0; + if (parameters['onPauseStart'].asBoolOr(false)) { + pauseTypeMask |= PauseTypeMasks.pauseOnStartMask; + } + if (parameters['onPauseExit'].asBoolOr(false)) { + pauseTypeMask |= PauseTypeMasks.pauseOnExitMask; + } + _requireUserPermissionToResumeMask = pauseTypeMask; + + // Check if isolates have been waiting for a user resume and resume any + // isolates that no longer need to wait for a user resume. + await maybeResumeIsolates(); + + return RPCResponses.success; + } + + /// Handles `getRequireUserPermissionToResume` requests. + /// + /// Returns an object indicating whether or not a `resume` request is + /// required for DDS to resume an isolate paused on start or exit. + Future> getRequireUserPermissionToResume( + DartDevelopmentServiceClient client, + json_rpc.Parameters parameters) async { + bool flagFromMask(int mask) => + _requireUserPermissionToResumeMask & mask != 0; + return { + 'type': 'ResumePermissionsRequired', + 'onPauseStart': flagFromMask(PauseTypeMasks.pauseOnStartMask), + 'onPauseExit': flagFromMask(PauseTypeMasks.pauseOnExitMask), + }; + } + Future> getCachedCpuSamples( json_rpc.Parameters parameters) async { final isolateId = parameters['isolateId'].asString; @@ -381,5 +475,6 @@ class IsolateManager { bool _initialized = false; final DartDevelopmentServiceImpl dds; final _mutex = Mutex(); - final Map isolates = {}; + int _requireUserPermissionToResumeMask = 0; + final isolates = {}; } diff --git a/pkg/dds/pubspec.yaml b/pkg/dds/pubspec.yaml index f295ed94cc8..01afa25a723 100644 --- a/pkg/dds/pubspec.yaml +++ b/pkg/dds/pubspec.yaml @@ -1,5 +1,5 @@ name: dds -version: 3.4.0 +version: 4.0.0 description: >- A library used to spawn the Dart Developer Service, used to communicate with a Dart VM Service instance. diff --git a/pkg/dds/test/client_resume_approvals_approve_then_disconnect_test.dart b/pkg/dds/test/client_resume_approvals_approve_then_disconnect_test.dart index 15f42f896da..98ea607d1c8 100644 --- a/pkg/dds/test/client_resume_approvals_approve_then_disconnect_test.dart +++ b/pkg/dds/test/client_resume_approvals_approve_then_disconnect_test.dart @@ -2,6 +2,7 @@ // 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 'package:dds_service_extensions/dds_service_extensions.dart'; import 'package:vm_service/vm_service.dart'; import 'client_resume_approvals_common.dart'; @@ -34,7 +35,7 @@ final test = [ // Give resume approval for client1 to ensure approval state is cleaned up // properly when both client1 and client2 have disconnected. - await client1.resume(isolateId); + await client1.readyToResume(isolateId); await hasPausedAtStart(service, isolate); // Once client1 is disconnected, we should still be paused. @@ -47,7 +48,7 @@ final test = [ client2.dispose(); await hasPausedAtStart(service, isolate); - await service.resume(isolateId); + await service.readyToResume(isolateId); }, hasStoppedAtExit, ]; diff --git a/pkg/dds/test/client_resume_approvals_disconnect_test.dart b/pkg/dds/test/client_resume_approvals_disconnect_test.dart index f5901cc957d..a2ae9fc797b 100644 --- a/pkg/dds/test/client_resume_approvals_disconnect_test.dart +++ b/pkg/dds/test/client_resume_approvals_disconnect_test.dart @@ -2,6 +2,7 @@ // 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 'package:dds_service_extensions/dds_service_extensions.dart'; import 'package:vm_service/vm_service.dart'; import 'client_resume_approvals_common.dart'; @@ -34,7 +35,7 @@ final test = [ // Send a resume request on the test client so we'll resume once the other // clients which require approval disconnect. - await service.resume(isolateId); + await service.readyToResume(isolateId); await hasPausedAtStart(service, isolate); // Once client1 is disconnected, we should still be paused. diff --git a/pkg/dds/test/client_resume_approvals_force_resume.dart b/pkg/dds/test/client_resume_approvals_force_resume.dart new file mode 100644 index 00000000000..3815dae3adb --- /dev/null +++ b/pkg/dds/test/client_resume_approvals_force_resume.dart @@ -0,0 +1,56 @@ +// 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 'package:dds_service_extensions/dds_service_extensions.dart'; +import 'package:vm_service/vm_service.dart'; + +import 'client_resume_approvals_common.dart'; +import 'common/service_test_common.dart'; +import 'common/test_helper.dart'; + +const String clientName = 'TestClient'; +const String otherClientName = 'OtherTestClient'; + +void fooBar() { + int i = 0; + print(i); +} + +final test = [ + // Multiple clients, different client names. + (VmService service, IsolateRef isolateRef) async { + final isolateId = isolateRef.id!; + final client1 = await createClient( + service: service, + clientName: clientName, + onPauseStart: true, + ); + // ignore: unused_local_variable + final client2 = await createClient( + service: service, + clientName: otherClientName, + onPauseStart: true, + ); + + await hasPausedAtStart(service, isolateRef); + await client1.requireUserPermissionToResume( + onPauseStart: true, + ); + + // Invocations of `resume` are considered to be resume requests made by the + // user and is treated as a force resume, ignoring any resume permissions + // set by clients. + await client1.resume(isolateId); + await hasStoppedAtExit(service, isolateRef); + }, +]; + +void main([args = const []]) => runIsolateTests( + args, + test, + 'client_resume_approvals_force_resume.dart', + testeeConcurrent: fooBar, + pauseOnStart: true, + pauseOnExit: true, + ); diff --git a/pkg/dds/test/client_resume_approvals_identical_names_test.dart b/pkg/dds/test/client_resume_approvals_identical_names_test.dart index 213c761a4ef..6183ff75d0f 100644 --- a/pkg/dds/test/client_resume_approvals_identical_names_test.dart +++ b/pkg/dds/test/client_resume_approvals_identical_names_test.dart @@ -2,6 +2,7 @@ // 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 'package:dds_service_extensions/dds_service_extensions.dart'; import 'package:vm_service/vm_service.dart'; import 'client_resume_approvals_common.dart'; @@ -29,7 +30,7 @@ final test = [ clientName: clientName, ); await hasPausedAtStart(service, isolateRef); - await resumeIsolate(client2, isolateRef); + await client2.readyToResume(isolateRef.id!); }, hasStoppedAtExit, ]; diff --git a/pkg/dds/test/client_resume_approvals_multiple_names_test.dart b/pkg/dds/test/client_resume_approvals_multiple_names_test.dart index 47a836904aa..6d2d897a09f 100644 --- a/pkg/dds/test/client_resume_approvals_multiple_names_test.dart +++ b/pkg/dds/test/client_resume_approvals_multiple_names_test.dart @@ -39,16 +39,16 @@ final test = [ ); await hasPausedAtStart(service, isolateRef); - await client2.resume(isolateId); + await client2.readyToResume(isolateId); await hasPausedAtStart(service, isolateRef); - await client1.resume(isolateId); + await client1.readyToResume(isolateId); await hasStoppedAtExit(service, isolateRef); await client2.requirePermissionToResume( onPauseExit: true, ); - await client1.resume(isolateId); + await client1.readyToResume(isolateId); await hasStoppedAtExit(service, isolateRef); - await client2.resume(isolateId); + await client2.readyToResume(isolateId); }, ]; diff --git a/pkg/dds/test/client_resume_approvals_name_change_test.dart b/pkg/dds/test/client_resume_approvals_name_change_test.dart index 6293f6b036b..a54b25eff94 100644 --- a/pkg/dds/test/client_resume_approvals_name_change_test.dart +++ b/pkg/dds/test/client_resume_approvals_name_change_test.dart @@ -41,7 +41,7 @@ final test = [ // Check that client3 can't resume the isolate on its own. await hasPausedAtStart(service, isolateRef); - await client3.resume(isolateId); + await client3.readyToResume(isolateId); await hasPausedAtStart(service, isolateRef); // Change the name of client1. Since client2 has the same name that client1 diff --git a/pkg/dds/test/client_resume_approvals_no_longer_require_permission.dart b/pkg/dds/test/client_resume_approvals_no_longer_require_permission.dart new file mode 100644 index 00000000000..6b7d8de74d6 --- /dev/null +++ b/pkg/dds/test/client_resume_approvals_no_longer_require_permission.dart @@ -0,0 +1,57 @@ +// 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 'package:dds_service_extensions/dds_service_extensions.dart'; +import 'package:vm_service/vm_service.dart'; + +import 'client_resume_approvals_common.dart'; +import 'common/service_test_common.dart'; +import 'common/test_helper.dart'; + +const String clientName = 'TestClient'; +const String otherClientName = 'OtherTestClient'; + +void fooBar() { + int i = 0; + print(i); +} + +final test = [ + (VmService service, IsolateRef isolateRef) async { + final isolateId = isolateRef.id!; + final client1 = await createClient( + service: service, + clientName: clientName, + onPauseStart: true, + ); + final client2 = await createClient( + service: service, + clientName: otherClientName, + onPauseStart: true, + ); + + await hasPausedAtStart(service, isolateRef); + + // When one client resumes, DDS waits to resume until the other client + // indicates it's ready. + await client2.readyToResume(isolateId); + await hasPausedAtStart(service, isolateRef); + + // If the only remaining client changes their resume permissions, DDS + // should check if the isolate should be resumed. In this case, the only + // other client requiring permission to resume has indicated it's ready + // so the isolate is resumed and pauses at exit. + await client1.requirePermissionToResume(onPauseStart: false); + await hasStoppedAtExit(service, isolateRef); + }, +]; + +void main([args = const []]) => runIsolateTests( + args, + test, + 'client_resume_approvals_no_longer_require_permission.dart', + testeeConcurrent: fooBar, + pauseOnStart: true, + pauseOnExit: true, + ); diff --git a/pkg/dds/test/client_resume_approvals_no_longer_require_user_permission.dart b/pkg/dds/test/client_resume_approvals_no_longer_require_user_permission.dart new file mode 100644 index 00000000000..0ad97b1a744 --- /dev/null +++ b/pkg/dds/test/client_resume_approvals_no_longer_require_user_permission.dart @@ -0,0 +1,65 @@ +// 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 'package:dds_service_extensions/dds_service_extensions.dart'; +import 'package:vm_service/vm_service.dart'; + +import 'client_resume_approvals_common.dart'; +import 'common/service_test_common.dart'; +import 'common/test_helper.dart'; + +const String clientName = 'TestClient'; +const String otherClientName = 'OtherTestClient'; + +void fooBar() { + int i = 0; + print(i); +} + +final test = [ + (VmService service, IsolateRef isolateRef) async { + final isolateId = isolateRef.id!; + final client1 = await createClient( + service: service, + clientName: clientName, + onPauseStart: true, + ); + final client2 = await createClient( + service: service, + clientName: otherClientName, + onPauseStart: true, + ); + + await hasPausedAtStart(service, isolateRef); + await client1.requireUserPermissionToResume( + onPauseStart: true, + ); + + // Both clients indicate they're ready to resume but the isolate won't + // resume until `resume` is invoked to indicate the user has triggered a + // resume. + await client2.readyToResume(isolateId); + await hasPausedAtStart(service, isolateRef); + await client1.readyToResume(isolateId); + await hasPausedAtStart(service, isolateRef); + + // If the user is no longer required to resume and all other clients have + // indicated they're ready to resume, the isolate should resume + // immediately. + await client1.requireUserPermissionToResume( + onPauseStart: false, + ); + + await hasStoppedAtExit(service, isolateRef); + }, +]; + +void main([args = const []]) => runIsolateTests( + args, + test, + 'client_resume_approvals_no_longer_require_user_permission.dart', + testeeConcurrent: fooBar, + pauseOnStart: true, + pauseOnExit: true, + ); diff --git a/pkg/dds/test/client_resume_approvals_reload_test.dart b/pkg/dds/test/client_resume_approvals_reload_test.dart index 3cc1dc3b531..38f5912ce9e 100644 --- a/pkg/dds/test/client_resume_approvals_reload_test.dart +++ b/pkg/dds/test/client_resume_approvals_reload_test.dart @@ -2,6 +2,7 @@ // 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 'package:dds_service_extensions/dds_service_extensions.dart'; import 'package:vm_service/vm_service.dart'; import 'client_resume_approvals_common.dart'; @@ -45,7 +46,7 @@ final test = [ (VmService service, IsolateRef isolateRef) async { final isolateId = isolateRef.id!; // Check that client2 can't resume the isolate on its own. - await client2.resume(isolateId); + await client2.readyToResume(isolateId); await hasStoppedPostRequest(service, isolateRef); await resumeIsolate(client1, isolateRef); }, diff --git a/pkg/dds/test/client_resume_approvals_require_user_permission.dart b/pkg/dds/test/client_resume_approvals_require_user_permission.dart new file mode 100644 index 00000000000..3d943aff42e --- /dev/null +++ b/pkg/dds/test/client_resume_approvals_require_user_permission.dart @@ -0,0 +1,76 @@ +// 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 'package:dds_service_extensions/dds_service_extensions.dart'; +import 'package:vm_service/vm_service.dart'; + +import 'client_resume_approvals_common.dart'; +import 'common/service_test_common.dart'; +import 'common/test_helper.dart'; + +const String clientName = 'TestClient'; +const String otherClientName = 'OtherTestClient'; + +void fooBar() { + int i = 0; + print(i); +} + +final test = [ + (VmService service, IsolateRef isolateRef) async { + final isolateId = isolateRef.id!; + final client1 = await createClient( + service: service, + clientName: clientName, + onPauseStart: true, + onPauseExit: true, + ); + final client2 = await createClient( + service: service, + clientName: otherClientName, + onPauseStart: true, + onPauseExit: true, + ); + + await hasPausedAtStart(service, isolateRef); + await client1.requireUserPermissionToResume( + onPauseStart: true, + onPauseExit: true, + ); + + // Both clients indicate they're ready to resume but the isolate won't + // resume until `resume` is invoked to indicate the user has triggered a + // resume. + await client2.readyToResume(isolateId); + await hasPausedAtStart(service, isolateRef); + await client1.readyToResume(isolateId); + await hasPausedAtStart(service, isolateRef); + + // Indicate the user is ready to resume and the isolate should run to + // completion. + await client1.resume(isolateId); + await hasStoppedAtExit(service, isolateRef); + + // Both clients indicate they're ready to resume but the isolate won't + // resume until `resume` is invoked to indicate the user has triggered a + // resume. + await client2.readyToResume(isolateId); + await hasStoppedAtExit(service, isolateRef); + await client1.readyToResume(isolateId); + await hasStoppedAtExit(service, isolateRef); + + await client1.resume(isolateId); + // We can't verify the process actually resumes with this test harness, so + // we're going to assume it did. + }, +]; + +void main([args = const []]) => runIsolateTests( + args, + test, + 'client_resume_approvals_require_user_permission.dart', + testeeConcurrent: fooBar, + pauseOnStart: true, + pauseOnExit: true, + ); diff --git a/pkg/dds_service_extensions/CHANGELOG.md b/pkg/dds_service_extensions/CHANGELOG.md index 3917f2d87a0..c57535d45aa 100644 --- a/pkg/dds_service_extensions/CHANGELOG.md +++ b/pkg/dds_service_extensions/CHANGELOG.md @@ -1,3 +1,9 @@ +# 2.0.0 +- Updated to DDS protocol 2.0. +- Added: + - `readyToResume` + - `requireUserPermissionToResume` + # 1.7.0 - Added: - `ClientName` diff --git a/pkg/dds_service_extensions/lib/dds_service_extensions.dart b/pkg/dds_service_extensions/lib/dds_service_extensions.dart index 02f7ed043bd..edada8ddf69 100644 --- a/pkg/dds_service_extensions/lib/dds_service_extensions.dart +++ b/pkg/dds_service_extensions/lib/dds_service_extensions.dart @@ -236,7 +236,9 @@ extension DdsExtension on VmService { /// This provides a way for the VM service to wait for approval to resume /// from some set of clients. This is useful for clients which want to /// perform some operation on an isolate after a pause without it being - /// resumed by another client. + /// resumed by another client. These clients should invoke [readyToResume] + /// instead of [VmService.resume] to indicate to DDS that they have finished + /// their work and the isolate can be resumed. /// /// If the [onPauseStart] parameter is `true`, isolates will not resume after /// pausing on start until the client sends a `resume` request and all other @@ -263,6 +265,11 @@ extension DdsExtension on VmService { /// resume approval for the current pause event, the isolate will be /// resumed if at least one other client has attempted to resume the /// isolate. + /// - Resume permission behavior can be bypassed using the [VmService.resume] + /// RPC, which is treated as a user-initiated resume that force resumes + /// the isolate. Tooling relying on resume permissions should use + /// [readyToResume] instead of [VmService.resume] to avoid force resuming + /// the isolate. Future requirePermissionToResume({ bool onPauseStart = false, bool onPauseReload = false, @@ -279,6 +286,52 @@ extension DdsExtension on VmService { ); } + /// The [readyToResume] RPC indicates to DDS that the current client is ready + /// to resume the isolate. + /// + /// If the current client requires that approval be given before resuming an + /// isolate, this method will: + /// + /// - Update the approval state for the isolate. + /// - Resume the isolate if approval has been given by all clients which + /// require approval. + /// + /// Throws a [SentinelException] if the isolate no longer exists. + Future readyToResume(String isolateId) async { + if (!(await _versionCheck(2, 0))) { + throw UnimplementedError('readyToResume requires DDS version 2.0'); + } + return _callHelper( + 'readyToResume', + isolateId: isolateId, + ); + } + + /// The [requireUserPermissionToResume] RPC notifies DDS if it should wait + /// for a [VmService.resume] request to resume isolates paused on start or + /// exit. + /// + /// This RPC should only be invoked by tooling which launched the target Dart + /// process and knows if the user indicated they wanted isolates paused on + /// start or exit. + Future requireUserPermissionToResume({ + bool onPauseStart = false, + bool onPauseExit = false, + }) async { + if (!(await _versionCheck(2, 0))) { + throw UnimplementedError( + 'requireUserPermissionToResume requires DDS version 2.0', + ); + } + return _callHelper( + 'requireUserPermissionToResume', + args: { + 'onPauseStart': onPauseStart, + 'onPauseExit': onPauseExit, + }, + ); + } + Future _versionCheck(int major, int minor) async { _ddsVersion ??= await getDartDevelopmentServiceVersion(); return ((_ddsVersion!.major == major && _ddsVersion!.minor! >= minor) || @@ -308,6 +361,10 @@ extension DdsExtension on VmService { addTypeFactory('CachedCpuSamples', CachedCpuSamples.parse); addTypeFactory('Size', Size.parse); addTypeFactory('ClientName', ClientName.parse); + addTypeFactory( + 'ResumePermissionsRequired', + ResumePermissionsRequired.parse, + ); _factoriesRegistered = true; } } @@ -442,3 +499,20 @@ class AvailableCachedCpuSamples extends Response { /// A [List] of [UserTag] names associated with CPU sample caches. final List cacheNames; } + +class ResumePermissionsRequired extends Response { + static ResumePermissionsRequired? parse(Map? json) => + json == null ? null : ResumePermissionsRequired._fromJson(json); + + ResumePermissionsRequired({ + required this.onPauseStart, + required this.onPauseExit, + }); + + ResumePermissionsRequired._fromJson(Map json) + : onPauseStart = json['onPauseStart'], + onPauseExit = json['onPauseExit']; + + final bool onPauseStart; + final bool onPauseExit; +} diff --git a/pkg/dds_service_extensions/lib/src/dds_service_extensions_base.dart b/pkg/dds_service_extensions/lib/src/dds_service_extensions_base.dart deleted file mode 100644 index e8a6f159017..00000000000 --- a/pkg/dds_service_extensions/lib/src/dds_service_extensions_base.dart +++ /dev/null @@ -1,6 +0,0 @@ -// TODO: Put public facing types in this file. - -/// Checks if you are awesome. Spoiler: you are. -class Awesome { - bool get isAwesome => true; -} diff --git a/pkg/dds_service_extensions/pubspec.yaml b/pkg/dds_service_extensions/pubspec.yaml index 414709c2d98..e9e3ff3254a 100644 --- a/pkg/dds_service_extensions/pubspec.yaml +++ b/pkg/dds_service_extensions/pubspec.yaml @@ -1,5 +1,5 @@ name: dds_service_extensions -version: 1.7.0 +version: 2.0.0 description: >- Extension methods for `package:vm_service`, used to make requests a Dart Development Service (DDS) instance. diff --git a/pkg/vm_service/test/get_supported_protocols_common.dart b/pkg/vm_service/test/get_supported_protocols_common.dart index 0f28c7f0f21..a7be12d5fd7 100644 --- a/pkg/vm_service/test/get_supported_protocols_common.dart +++ b/pkg/vm_service/test/get_supported_protocols_common.dart @@ -25,7 +25,7 @@ VMTest expectedProtocolTest( expect(protocols.length, expectedLength); for (final protocol in protocols) { expect(expectedProtocols.contains(protocol.protocolName), true); - expect(protocol.minor, greaterThan(0)); + expect(protocol.minor, greaterThanOrEqualTo(0)); expect(protocol.major, greaterThan(0)); } };