[dds/dap] Don't handle isolate startup until isolate is in PauseStart state
While investigating the flaky test at https://github.com/dart-lang/sdk/issues/60187#issuecomment-3431336127 I found that we had two paths into `handleThreadStartup` - one which explicitly sent a "pause-on-entry" event, and one that did not. The method sets a flag so it only ever runs once.
This means depending on which instance of `handleThreadStartup` occurs, the client may or may not get the pause-on-entry event.
The case where it would is where the isolate already had a pause-start event when we found it. The case where it would not, is if the isolate did not have a pause event (for example it was None because the isolate was just being created).
I can't come up with a good explanation for why we would try to pre-emptively run startup for an isolate we just discovered with no pauseEvent, so I have removed this branch on the assumption that we would always then see a `PauseStart` event for the isolate and then handled it through the same code path (and therefore, always sent the `pause-on-entry` event prior to calling readyToResume).
Fixes https://github.com/dart-lang/sdk/issues/60187
Change-Id: I213e9865480942b074d4d8470703377ea7ddb519
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/457040
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md
index 03b0e6f..32cc8aa 100644
--- a/pkg/dds/CHANGELOG.md
+++ b/pkg/dds/CHANGELOG.md
@@ -1,3 +1,7 @@
+# 5.2.0-wip
+
+- [DAP] `Stopped(reason: 'entry')` events will no longer be lost if an isolate has not yet reached the `PauseStart` state when connecting to the VM.
+
# 5.1.0
- Update to version 2.1 of the DDS protocol.
- Deprecate `getAvailableCachedCpuSamples` and `getCachedCpuSamples` RPCs.
diff --git a/pkg/dds/lib/src/dap/adapters/dart.dart b/pkg/dds/lib/src/dap/adapters/dart.dart
index a1c53b2..70bd224 100644
--- a/pkg/dds/lib/src/dap/adapters/dart.dart
+++ b/pkg/dds/lib/src/dap/adapters/dart.dart
@@ -831,21 +831,16 @@
final pauseEventKind = isolate.runnable ?? false
? vm.EventKind.kIsolateRunnable
: vm.EventKind.kIsolateStart;
- final thread =
- await isolateManager.registerIsolate(isolate, pauseEventKind);
+ await isolateManager.registerIsolate(isolate, pauseEventKind);
// If the Isolate already has a Pause event we can give it to the
// IsolateManager to handle (if it's PausePostStart it will re-configure
- // the isolate before resuming), otherwise we can just resume it (if it's
- // runnable - otherwise we'll handle this when it becomes runnable in an
- // event later).
+ // the isolate before resuming), otherwise wait for the PauseStart event
+ // to arrive and then this will be done.
if (isolate.pauseEvent?.kind?.startsWith('Pause') ?? false) {
await isolateManager.handleEvent(
isolate.pauseEvent!,
);
- } else if (isolate.runnable == true) {
- await isolateManager.handleThreadStartup(thread,
- sendStoppedOnEntry: false);
}
}));
}
diff --git a/pkg/dds/lib/src/dap/isolate_manager.dart b/pkg/dds/lib/src/dap/isolate_manager.dart
index bfc2f49..aad6681 100644
--- a/pkg/dds/lib/src/dap/isolate_manager.dart
+++ b/pkg/dds/lib/src/dap/isolate_manager.dart
@@ -694,7 +694,7 @@
thread.startupHandled = true;
await readyToResumeThread(thread.threadId);
} else if (eventKind == vm.EventKind.kPauseStart) {
- handleThreadStartup(thread, sendStoppedOnEntry: true);
+ handleThreadStartup(thread);
} else {
// PauseExit, PauseBreakpoint, PauseInterrupted, PauseException
var reason = 'pause';
@@ -765,10 +765,7 @@
/// Handles thread startup if it has not already been handled.
///
/// This includes sending Stopped-on-Entry and sending a readyToResume.
- Future<void> handleThreadStartup(
- ThreadInfo thread, {
- required bool sendStoppedOnEntry,
- }) async {
+ Future<void> handleThreadStartup(ThreadInfo thread) async {
// Don't resume from a PauseStart if this has already happened (see
// comments on [thread.startupHandled]).
if (thread.startupHandled) {
@@ -776,12 +773,11 @@
}
thread.startupHandled = true;
+
// Send a Stopped event to inform the client UI the thread is paused and
// declare that we are ready to resume (which might result in an
// immediate resume).
- if (sendStoppedOnEntry) {
- sendStoppedOnEntryEvent(thread);
- }
+ sendStoppedOnEntryEvent(thread);
await readyToResumeThread(thread.threadId);
}
diff --git a/pkg/dds/pubspec.yaml b/pkg/dds/pubspec.yaml
index dd8c6b0..511443d 100644
--- a/pkg/dds/pubspec.yaml
+++ b/pkg/dds/pubspec.yaml
@@ -1,5 +1,5 @@
name: dds
-version: 5.1.0
+version: 5.2.0-wip
description: >-
A library used to spawn the Dart Developer Service, used to communicate with
a Dart VM Service instance.
