From 29c350dfe963ff07d3129f65866ce682ebdd5260 Mon Sep 17 00:00:00 2001
From: Danny Tuppeny <danny@tuppeny.com>
Date: Wed, 19 Mar 2025 07:32:19 -0700
Subject: [PATCH] [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>
---
 pkg/dds/lib/src/dap/isolate_manager.dart   |  7 +++-
 pkg/dds/test/dap/isolate_manager_test.dart | 22 ++++++++++++
 pkg/dds/test/dap/mocks.dart                | 42 +++++++++++++++++++++-
 3 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/pkg/dds/lib/src/dap/isolate_manager.dart b/pkg/dds/lib/src/dap/isolate_manager.dart
index a029480e708..6d90f2e5fbc 100644
--- a/pkg/dds/lib/src/dap/isolate_manager.dart
+++ b/pkg/dds/lib/src/dap/isolate_manager.dart
@@ -645,7 +645,12 @@ class IsolateManager {
     // 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 74871d42138..0751f62a3af 100644
--- a/pkg/dds/test/dap/isolate_manager_test.dart
+++ b/pkg/dds/test/dap/isolate_manager_test.dart
@@ -144,5 +144,27 @@ main() {
       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 3b6b81b8950..7a4c3a707ab 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 @@ class MockVmService implements VmService {
   /// 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,
@@ -213,6 +217,30 @@ class MockVmService implements VmService {
     );
   }
 
+  @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, {
@@ -224,6 +252,18 @@ class MockVmService implements VmService {
         ? 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