[dds/dap] Ensure DAP always sends resume on PausePostRequest
The change at https://dart-review.googlesource.com/c/sdk/+/410760 to fix a race incorrectly assumed that once we had handled startup for a thread, we would never need to send an automatic resume again. However this was not the case - after a hot reload, we need to resume the thread even though we had technically already handled startup.
This is essentially a partial revert of 84e6ed0784c679c03ec6380a4aed6aa78eab2c24, with a test to verify we trigger readyToResume on PausePostRequest.
Change-Id: I1e171043f04f38dd6f52e1692d9257d34e5248be
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/416580
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Derek Xu <derekx@google.com>
diff --git a/pkg/dds/lib/src/dap/isolate_manager.dart b/pkg/dds/lib/src/dap/isolate_manager.dart
index a029480..6d90f2e 100644
--- a/pkg/dds/lib/src/dap/isolate_manager.dart
+++ b/pkg/dds/lib/src/dap/isolate_manager.dart
@@ -645,7 +645,12 @@
// after a hot restart.
if (eventKind == vm.EventKind.kPausePostRequest) {
await _configureIsolate(thread);
- await handleThreadStartup(thread, sendStoppedOnEntry: false);
+
+ // We always want to resume here regardless of whether startupHandled was
+ // already `true` (because that might be from before the reload).
+ // Therefore set the flag and resume always.
+ thread.startupHandled = true;
+ await readyToResumeThread(thread.threadId);
} else if (eventKind == vm.EventKind.kPauseStart) {
handleThreadStartup(thread, sendStoppedOnEntry: true);
} else {
diff --git a/pkg/dds/test/dap/isolate_manager_test.dart b/pkg/dds/test/dap/isolate_manager_test.dart
index 74871d4..0751f62 100644
--- a/pkg/dds/test/dap/isolate_manager_test.dart
+++ b/pkg/dds/test/dap/isolate_manager_test.dart
@@ -144,5 +144,27 @@
expect(receivedUri, 'scheme://file/path.dart');
expect(resolved, Uri.file('/returned/file/path.dart'));
});
+
+ test('sends resume after kPausePostRequest', () async {
+ // Trigger initial startup.
+ await adapter.isolateManager.handleEvent(Event(
+ isolate: adapter.mockService.isolate1,
+ kind: EventKind.kIsolateRunnable));
+ isolateManager.threads[0].startupHandled = true;
+
+ // Clear any previously called methods.
+ adapter.mockService.calledMethods.clear();
+
+ // Now simulate a hot reload causing the isolate to pause again.
+ await adapter.isolateManager.handleEvent(Event(
+ isolate: adapter.mockService.isolate1,
+ kind: EventKind.kPausePostRequest));
+
+ // Ensure we called readyToResume.
+ expect(adapter.mockService.calledMethods.map((cm) => cm.method),
+ contains('readyToResume'));
+
+ // Expect it was resumed.
+ });
});
}
diff --git a/pkg/dds/test/dap/mocks.dart b/pkg/dds/test/dap/mocks.dart
index 3b6b81b..7a4c3a7 100644
--- a/pkg/dds/test/dap/mocks.dart
+++ b/pkg/dds/test/dap/mocks.dart
@@ -5,7 +5,7 @@
import 'dart:async';
import 'package:dap/dap.dart' as dap;
-import 'package:dds/dap.dart';
+import 'package:dds/dap.dart' hide Response;
import 'package:dds/dds.dart';
import 'package:dds/dds_launcher.dart';
import 'package:dds/src/dap/adapters/dart_cli_adapter.dart';
@@ -168,6 +168,10 @@
/// tests.
final List<String> requests = [];
+ /// A list of methods that were called on the Mock VM Service.
+ List<({String method, String? isolateId, Map<String, dynamic>? args})>
+ calledMethods = [];
+
@override
Future<Success> setLibraryDebuggable(
String isolateId,
@@ -214,6 +218,30 @@
}
@override
+ Future<Response> callMethod(
+ String method, {
+ String? isolateId,
+ Map<String, dynamic>? args,
+ }) async {
+ calledMethods.add((method: method, isolateId: isolateId, args: args));
+
+ // Some DDS methods are implemented as extensions so we can't override
+ // them in the mock, we need to override callMethod and handle them here
+ // (and provide their responses) instead.
+
+ if (method == 'getDartDevelopmentServiceVersion') {
+ return Version(major: 2, minor: 0);
+ } else if (method == 'readyToResume') {
+ isolateId ??= args != null ? args['isolateId'] : null;
+ return {isolate1.id, isolate2.id}.contains(isolateId)
+ ? Success()
+ : throw SentinelException.parse(method, {});
+ }
+
+ throw 'MockVmService does not handle $method ($isolateId, $args)';
+ }
+
+ @override
Future<Success> resume(
String isolateId, {
String? step,
@@ -224,6 +252,18 @@
? Success()
: throw SentinelException.parse('resume', {});
}
+
+ @override
+ Future<Success> setIsolatePauseMode(
+ String isolateId, {
+ /*ExceptionPauseMode*/ String? exceptionPauseMode,
+ bool? shouldPauseOnExit,
+ }) async {
+ // Do nothing, just pretend.
+ return {isolate1.id, isolate2.id}.contains(isolateId)
+ ? Success()
+ : throw SentinelException.parse('setIsolatePauseMode', {});
+ }
}
class MockDartDevelopmentServiceLauncher
