[DAP] Set `requirePermissionToResume` and `requireUserPermissionToResume` for `onPauseStart` and `onPauseExit` so that DDS waits for DAP's permission before resuming the isolate.
Bug: https://github.com/dart-lang/sdk/issues/54843
Change-Id: I12686d6c683983b7af5a3148681410ff9c684343
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350649
Reviewed-by: Ben Konyi <bkonyi@google.com>
Commit-Queue: Elliott Brooks <elliottbrooks@google.com>
diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md
index 64769fd..a42cda4 100644
--- a/pkg/dds/CHANGELOG.md
+++ b/pkg/dds/CHANGELOG.md
@@ -2,6 +2,7 @@
- [DAP]: Fixed an issue where breakpoint `changed` events might contain incorrect location information when new isolates are created, causing breakpoints to appear to move in the editor.
- [DAP]: For consistency with other values, automatic `toString()` invocations for debugger views no longer expand long strings and instead show truncated values. Full values continue to be returned for evaluation (`context=="repl"`) and when copying to the clipboard (`context=="clipboard"`).
- [DAP]: Improved handling of sentinel responses when building `variables` responses. This prevents entire map/list requests from failing when only some values inside are sentinels.
+- [DAP] Set `requirePermissionToResume` and `requireUserPermissionToResume` for `onPauseStart` and `onPauseExit` so that DDS waits for DAP's permission before resuming the isolate.
# 4.2.0
- [DAP] All `OutputEvent`s are now scanned for stack frames to attach `source` metadata to. The [parseStackFrames] parameter for `sendOutput` is ignored and deprecated.
diff --git a/pkg/dds/lib/src/dap/adapters/dart.dart b/pkg/dds/lib/src/dap/adapters/dart.dart
index ab27a7f..15b231b 100644
--- a/pkg/dds/lib/src/dap/adapters/dart.dart
+++ b/pkg/dds/lib/src/dap/adapters/dart.dart
@@ -8,6 +8,7 @@
import 'package:collection/collection.dart';
import 'package:dap/dap.dart';
+import 'package:dds_service_extensions/dds_service_extensions.dart';
import 'package:json_rpc_2/error_code.dart' as json_rpc_errors;
import 'package:meta/meta.dart';
import 'package:path/path.dart' as path;
@@ -434,12 +435,19 @@
/// vs 'detached').
bool isDetaching = false;
- /// Whether isolates that pause in the PauseExit state should be automatically
- /// resumed after any in-process log events have completed.
+ /// Whether this adapter set the --pause-isolates-on-start flag, specifying
+ /// that isolates should pause on starting.
///
/// Normally this will be true, but it may be set to false if the user
- /// also manually passes pause-isolates-on-exit.
- bool resumeIsolatesAfterPauseExit = true;
+ /// also manually passed the --pause-isolates-on-start flag.
+ bool pauseIsolatesOnStartSetByDap = true;
+
+ /// Whether this adapter set the --pause-isolates-on-exit flag, specifying
+ /// that isolates should pause on exiting.
+ ///
+ /// Normally this will be true, but it may be set to false if the user
+ /// also manually passed the --pause-isolates-on-exit flag.
+ bool pauseIsolatesOnExitSetByDap = true;
/// A [Future] that completes when the last queued OutputEvent has been sent.
///
@@ -559,10 +567,6 @@
isAttach = true;
_subscribeToOutputStreams = true;
- // When attaching to a process, suppress auto-resuming isolates until the
- // first time the user resumes anything.
- isolateManager.autoResumeStartingIsolates = false;
-
// Common setup.
await _prepareForLaunchOrAttach(null);
@@ -702,6 +706,7 @@
// Let the subclass do any existing setup once we have a connection.
await debuggerConnected(vmInfo);
+ await _configureIsolateSettings(vmService);
await _withErrorHandling(
() => _configureExistingIsolates(vmService, vmInfo),
);
@@ -756,6 +761,68 @@
return DapProgressReporter.start(this, id, title, message: message);
}
+ Future<void> _configureIsolateSettings(
+ vm.VmService vmService,
+ ) async {
+ // If this is an attach workflow, check whether pause_isolates_on_start or
+ // pause_isolates_on_exit were already set, and if not set them (note: this
+ // is already done as part of the launch workflow):
+ if (isAttach) {
+ const pauseIsolatesOnStart = 'pause_isolates_on_start';
+ const pauseIsolatesOnExit = 'pause_isolates_on_exit';
+ final flags = (await vmService.getFlagList()).flags ?? <vm.Flag>[];
+ for (final flag in flags) {
+ final flagName = flag.name;
+ final isPauseIsolatesFlag =
+ flagName == pauseIsolatesOnStart || flagName == pauseIsolatesOnExit;
+ if (flagName == null || !isPauseIsolatesFlag) continue;
+
+ if (flag.valueAsString == 'true') {
+ if (flagName == pauseIsolatesOnStart) {
+ pauseIsolatesOnStartSetByDap = false;
+ }
+ if (flagName == pauseIsolatesOnExit) {
+ pauseIsolatesOnExitSetByDap = false;
+ }
+ } else {
+ _setVmFlagTo(vmService, flagName: flagName, valueAsString: 'true');
+ }
+ }
+ }
+
+ try {
+ // Make sure DDS waits for DAP to be ready to resume before forwarding
+ // resume requests to the VM Service:
+ await vmService.requirePermissionToResume(
+ onPauseStart: true,
+ onPauseExit: true,
+ );
+
+ // Specify whether DDS should wait for a user-initiated resume as well as a
+ // DAP-initiated resume:
+ await vmService.requireUserPermissionToResume(
+ onPauseStart: !pauseIsolatesOnStartSetByDap,
+ onPauseExit: !pauseIsolatesOnExitSetByDap,
+ );
+ } catch (e) {
+ // If DDS is not enabled, calling these DDS service extensions will fail.
+ // Therefore catch and log any errors.
+ logger?.call('Failure configuring isolate settings: $e');
+ }
+ }
+
+ Future<void> _setVmFlagTo(
+ vm.VmService vmService, {
+ required String flagName,
+ required String valueAsString,
+ }) async {
+ try {
+ await vmService.setFlag(flagName, valueAsString);
+ } catch (e) {
+ logger?.call('Failed to to set VM flag $flagName to $valueAsString: $e');
+ }
+ }
+
/// Process any existing isolates that may have been created before the
/// streams above were set up.
Future<void> _configureExistingIsolates(
@@ -775,8 +842,7 @@
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
@@ -788,13 +854,7 @@
isolate.pauseEvent!,
);
} else if (isolate.runnable == true) {
- // If requested, automatically resume. Otherwise send a Stopped event to
- // inform the client UI the thread is paused.
- if (isolateManager.autoResumeStartingIsolates) {
- await isolateManager.resumeIsolate(isolate);
- } else {
- isolateManager.sendStoppedOnEntryEvent(thread.threadId);
- }
+ await isolateManager.readyToResumeIsolate(isolate);
}
}));
}
@@ -2352,11 +2412,9 @@
// We pause isolates on exit to allow requests for resolving URIs in
// stderr call stacks, so when we see an isolate pause, wait for any
// pending logs and then resume it (so it exits).
- if (resumeIsolatesAfterPauseExit &&
- eventKind == vm.EventKind.kPauseExit &&
- isolate != null) {
+ if (eventKind == vm.EventKind.kPauseExit && isolate != null) {
await _waitForPendingOutputEvents();
- await isolateManager.resumeIsolate(isolate);
+ await isolateManager.readyToResumeIsolate(isolate);
}
}
diff --git a/pkg/dds/lib/src/dap/adapters/dart_cli_adapter.dart b/pkg/dds/lib/src/dap/adapters/dart_cli_adapter.dart
index 77913c1..8c14d7a 100644
--- a/pkg/dds/lib/src/dap/adapters/dart_cli_adapter.dart
+++ b/pkg/dds/lib/src/dap/adapters/dart_cli_adapter.dart
@@ -11,6 +11,7 @@
import 'package:path/path.dart' as path;
import 'package:vm_service/vm_service.dart' as vm;
+import '../utils.dart';
import 'dart.dart';
import 'mixins.dart';
@@ -68,14 +69,6 @@
terminatePids(ProcessSignal.sigkill);
}
- /// Checks whether [flag] is in [args], allowing for both underscore and
- /// dash format.
- bool _containsVmFlag(List<String> args, String flag) {
- final flagUnderscores = flag.replaceAll('-', '_');
- final flagDashes = flag.replaceAll('_', '-');
- return args.contains(flagUnderscores) || args.contains(flagDashes);
- }
-
@override
Future<void> launchImpl() {
throw UnsupportedError(
@@ -115,7 +108,6 @@
...?args.vmAdditionalArgs,
if (debug) ...[
'--enable-vm-service=${args.vmServicePort ?? 0}${ipv6 ? '/::1' : ''}',
- '--pause_isolates_on_start',
if (!enableAuthCodes) '--disable-service-auth-codes'
],
'--disable-dart-dev',
@@ -128,12 +120,20 @@
final toolArgs = args.toolArgs ?? [];
if (debug) {
// If the user has explicitly set pause-isolates-on-exit we need to
- // not add it ourselves, and disable auto-resuming.
- if (_containsVmFlag(toolArgs, '--pause_isolates_on_exit')) {
- resumeIsolatesAfterPauseExit = false;
+ // not add it ourselves, and specify that we didn't set it.
+ if (containsVmFlag(toolArgs, '--pause_isolates_on_exit')) {
+ pauseIsolatesOnExitSetByDap = false;
} else {
vmArgs.add('--pause_isolates_on_exit');
}
+
+ // If the user has explicitly set pause-isolates-on-start we need to
+ // not add it ourselves, and specify that we didn't set it.
+ if (containsVmFlag(toolArgs, '--pause_isolates_on_start')) {
+ pauseIsolatesOnStartSetByDap = false;
+ } else {
+ vmArgs.add('--pause_isolates_on_start');
+ }
}
// Handle customTool and deletion of any arguments for it.
diff --git a/pkg/dds/lib/src/dap/adapters/dart_test_adapter.dart b/pkg/dds/lib/src/dap/adapters/dart_test_adapter.dart
index f9d66c1..f894136 100644
--- a/pkg/dds/lib/src/dap/adapters/dart_test_adapter.dart
+++ b/pkg/dds/lib/src/dap/adapters/dart_test_adapter.dart
@@ -10,6 +10,7 @@
import 'package:vm_service/vm_service.dart' as vm;
import '../stream_transformers.dart';
+import '../utils.dart';
import 'dart.dart';
import 'mixins.dart';
@@ -78,17 +79,32 @@
.then((uri) => connectDebugger(uri)));
}
- final vmArgs = <String>[
+ var vmArgs = <String>[
...?args.vmAdditionalArgs,
if (debug) ...[
'--enable-vm-service=${args.vmServicePort ?? 0}${ipv6 ? '/::1' : ''}',
- '--pause_isolates_on_start',
if (!enableAuthCodes) '--disable-service-auth-codes'
],
if (debug && vmServiceInfoFile != null) ...[
'-DSILENT_VM_SERVICE=true',
'--write-service-info=${Uri.file(vmServiceInfoFile.path)}'
],
+ ];
+
+ final toolArgs = args.toolArgs ?? [];
+ if (debug) {
+ // If the user has explicitly set pause-isolates-on-start we need to
+ // not add it ourselves, and specify that we didn't set it.
+ if (containsVmFlag(toolArgs, '--pause_isolates_on_start') ||
+ containsVmFlag(vmArgs, '--pause_isolates_on_start')) {
+ pauseIsolatesOnStartSetByDap = false;
+ } else {
+ vmArgs.add('--pause_isolates_on_start');
+ }
+ }
+
+ vmArgs = [
+ ...vmArgs,
// TODO(dantup): This should be changed from "dart run test:test" to
// "dart test" once the start-paused flags are working correctly.
// Currently they start paused but do not write the vm-service-info file
diff --git a/pkg/dds/lib/src/dap/isolate_manager.dart b/pkg/dds/lib/src/dap/isolate_manager.dart
index 893f520..8f77086 100644
--- a/pkg/dds/lib/src/dap/isolate_manager.dart
+++ b/pkg/dds/lib/src/dap/isolate_manager.dart
@@ -7,6 +7,7 @@
import 'package:collection/collection.dart';
import 'package:dap/dap.dart';
+import 'package:dds_service_extensions/dds_service_extensions.dart';
import 'package:vm_service/vm_service.dart' as vm;
import '../rpc_error_codes.dart';
@@ -62,16 +63,6 @@
/// [debugExternalPackageLibraries] in one step.
bool debugExternalPackageLibraries = true;
- /// Whether to automatically resume new isolates after configuring them.
- ///
- /// This setting is almost always `true` because isolates are paused only so
- /// we can configure them (send breakpoints, pause-on-exceptions,
- /// setLibraryDebuggables) without races. It is set to `false` during the
- /// initial connection of an `attachRequest` to allow paused isolates to
- /// remain paused. In this case, it will be automatically re-set to `true` the
- /// first time the user resumes.
- bool autoResumeStartingIsolates = true;
-
/// Tracks breakpoints last provided by the client so they can be sent to new
/// isolates that appear after initial breakpoints were sent.
final Map<String, List<ClientBreakpoint>> _clientBreakpointsByUri = {};
@@ -277,6 +268,17 @@
await resumeThread(thread.threadId);
}
+ Future<void> readyToResumeIsolate(vm.IsolateRef isolateRef) async {
+ final isolateId = isolateRef.id!;
+
+ final thread = _threadsByIsolateId[isolateId];
+ if (thread == null) {
+ return;
+ }
+
+ await readyToResumeThread(thread.threadId);
+ }
+
/// Resumes (or steps) an isolate using its client [threadId].
///
/// If the isolate is not paused, or already has a pending resume request
@@ -289,6 +291,16 @@
await _resume(threadId, resumeType: resumeType);
}
+ /// Resumes an isolate using its client [threadId].
+ ///
+ /// CAUTION: This should only be used for a tool-initiated resume, not a user-
+ /// initiated resume.
+ ///
+ /// See: https://pub.dev/documentation/dds_service_extensions/latest/dds_service_extensions/DdsExtension/readyToResume.html
+ Future<void> readyToResumeThread(int threadId) async {
+ await _readyToResume(threadId);
+ }
+
/// Rewinds an isolate to an earlier frame using its client [threadId].
///
/// If the isolate is not paused, or already has a pending resume request
@@ -301,7 +313,8 @@
);
}
- /// Resumes (or steps) an isolate using its client [threadId].
+ /// Resumes (or steps) an isolate using its client [threadId] on behalf
+ /// of the user.
///
/// If the isolate is not paused, or already has a pending resume request
/// in-flight, a request will not be sent.
@@ -316,11 +329,6 @@
String? resumeType,
int? frameIndex,
}) async {
- // The first time a user resumes a thread is our signal that the app is now
- // "running" and future isolates can be auto-resumed. This only affects
- // attach, as it's already `true` for launch requests.
- autoResumeStartingIsolates = true;
-
final thread = _threadsByThreadId[threadId];
if (thread == null) {
if (isInvalidThreadId(threadId)) {
@@ -336,7 +344,7 @@
// Check this thread hasn't already been resumed by another handler in the
// meantime (for example if the user performs a hot restart or something
// while we processing some previous events).
- if (!thread.paused || thread.hasPendingResume) {
+ if (!thread.paused || thread.hasPendingUserResume) {
return;
}
@@ -350,7 +358,7 @@
// we can drop them to save memory.
thread.clearStoredData();
- thread.hasPendingResume = true;
+ thread.hasPendingUserResume = true;
try {
await _adapter.vmService?.resume(
thread.isolate.id!,
@@ -369,7 +377,57 @@
rethrow;
}
} finally {
- thread.hasPendingResume = false;
+ thread.hasPendingUserResume = false;
+ }
+ }
+
+ /// Resumes an isolate using its client [threadId].
+ ///
+ /// CAUTION: This should only be used for a tool-initiated resume, not a user-
+ /// initiated resume.
+ ///
+ /// See: https://pub.dev/documentation/dds_service_extensions/latest/dds_service_extensions/DdsExtension/readyToResume.html
+ Future<void> _readyToResume(int threadId) async {
+ final thread = _threadsByThreadId[threadId];
+ if (thread == null) {
+ if (isInvalidThreadId(threadId)) {
+ throw DebugAdapterException('Thread $threadId was not found');
+ } else {
+ // Otherwise, this thread has exited and we don't need to do anything.
+ // It's possible another debugger unpaused or we're shutting down and
+ // the VM has terminated it.
+ return;
+ }
+ }
+
+ // When we're resuming, all stored objects become invalid and we can drop
+ // to save memory.
+ thread.clearStoredData();
+
+ try {
+ thread.hasPendingDapResume = true;
+ await _adapter.vmService?.readyToResume(thread.isolate.id!);
+ } on UnimplementedError {
+ // Fallback to a regular resume if the DDS version doesn't support
+ // `readyToResume`:
+ return _resume(threadId);
+ } on vm.SentinelException {
+ // It's possible during these async requests that the isolate went away
+ // (for example a shutdown/restart) and we no longer care about
+ // resuming it.
+ } on vm.RPCError catch (e) {
+ if (e.code == RpcErrorCodes.kIsolateMustBePaused) {
+ // It's possible something else resumed the thread (such as if another
+ // debugger is attached), we can just continue.
+ } else if (e.code == RpcErrorCodes.kMethodNotFound) {
+ // Fallback to a regular resume if the DDS service extension isn't
+ // available:
+ return _resume(threadId);
+ } else {
+ rethrow;
+ }
+ } finally {
+ thread.hasPendingDapResume = false;
}
}
@@ -406,8 +464,9 @@
bool isInvalidThreadId(int threadId) => threadId >= _nextThreadNumber;
/// Sends an event informing the client that a thread is stopped at entry.
- void sendStoppedOnEntryEvent(int threadId) {
- _adapter.sendEvent(StoppedEventBody(reason: 'entry', threadId: threadId));
+ void sendStoppedOnEntryEvent(ThreadInfo thread) {
+ _adapter.sendEvent(
+ StoppedEventBody(reason: 'entry', threadId: thread.threadId));
}
/// Records breakpoints for [uri].
@@ -557,11 +616,9 @@
///
/// For [vm.EventKind.kPausePostRequest] which occurs after a restart, the
/// isolate will be re-configured (pause-exception behaviour, debuggable
- /// libraries, breakpoints) and then (if [autoResumeStartingIsolates] is
- /// `true`) resumed.
+ /// libraries, breakpoints) and we'll declare we are ready to resume.
///
- /// For [vm.EventKind.kPauseStart] and [autoResumeStartingIsolates] is `true`,
- /// the isolate will be resumed.
+ /// For [vm.EventKind.kPauseStart] we'll declare we are ready to resume.
///
/// For breakpoints with conditions that are not met and for logpoints, the
/// isolate will be automatically resumed.
@@ -586,21 +643,17 @@
// after a hot restart.
if (eventKind == vm.EventKind.kPausePostRequest) {
await _configureIsolate(thread);
- if (autoResumeStartingIsolates) {
- await resumeThread(thread.threadId);
- }
+ await readyToResumeThread(thread.threadId);
} else if (eventKind == vm.EventKind.kPauseStart) {
// Don't resume from a PauseStart if this has already happened (see
// comments on [thread.hasBeenStarted]).
if (!thread.startupHandled) {
thread.startupHandled = true;
- // If requested, automatically resume. Otherwise send a Stopped event to
- // inform the client UI the thread is paused.
- if (autoResumeStartingIsolates) {
- await resumeThread(thread.threadId);
- } else {
- sendStoppedOnEntryEvent(thread.threadId);
- }
+ // 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).
+ sendStoppedOnEntryEvent(thread);
+ await readyToResumeThread(thread.threadId);
}
} else {
// PauseExit, PauseBreakpoint, PauseInterrupted, PauseException
@@ -643,6 +696,8 @@
reason = 'step';
} else if (eventKind == vm.EventKind.kPauseException) {
reason = 'exception';
+ } else if (eventKind == vm.EventKind.kPauseExit) {
+ reason = 'exit';
}
// If we stopped at an exception, capture the exception instance so we
@@ -671,6 +726,11 @@
final isolate = event.isolate!;
final thread = _threadsByIsolateId[isolate.id!];
if (thread != null) {
+ // When a thread is resumed, we must inform the client. This is not
+ // necessary when the user has clicked Continue because it is implied.
+ // However, resume events can now be triggered by other things (eg. other
+ // in other IDEs or DevTools) so we must notify the client.
+ _adapter.sendEvent(ContinuedEventBody(threadId: thread.threadId));
thread.paused = false;
thread.pauseEvent = null;
thread.exceptionReference = null;
@@ -1057,6 +1117,11 @@
var runnable = false;
var atAsyncSuspension = false;
int? exceptionReference;
+
+ /// Whether this thread is currently known to be paused in the VM.
+ ///
+ /// Because requests are async, this is not guaranteed to be always correct
+ /// but should represent the state based on the latest VM events.
var paused = false;
/// Tracks whether an isolates startup routine has been handled.
@@ -1096,9 +1161,13 @@
/// Values are file-like URIs (file: or similar, such as dart-macro+file:).
final _resolvedPaths = <String, Future<Uri?>>{};
- /// Whether this isolate has an in-flight resume request that has not yet
- /// been responded to.
- var hasPendingResume = false;
+ /// Whether this isolate has an in-flight user-initiated resume request that
+ /// has not yet been responded to.
+ var hasPendingUserResume = false;
+
+ /// Whether this isolate has an in-flight DAP (readyToResume) resume request
+ /// that has not yet been responded to.
+ var hasPendingDapResume = false;
ThreadInfo(this._manager, this.threadId, this.isolate);
diff --git a/pkg/dds/lib/src/dap/utils.dart b/pkg/dds/lib/src/dap/utils.dart
index d50a06a..5536345 100644
--- a/pkg/dds/lib/src/dap/utils.dart
+++ b/pkg/dds/lib/src/dap/utils.dart
@@ -98,3 +98,11 @@
tryParseFrame((line) => stack.Frame.parseIE(line)) ??
tryParseFrame((line) => stack.Frame.parseFriendly(line));
}
+
+/// Checks whether [flag] is in [args], allowing for both underscore and
+/// dash format.
+bool containsVmFlag(List<String> args, String flag) {
+ final flagUnderscores = flag.replaceAll('-', '_');
+ final flagDashes = flag.replaceAll('_', '-');
+ return args.contains(flagUnderscores) || args.contains(flagDashes);
+}
diff --git a/pkg/dds/test/dap/integration/dart_test_test.dart b/pkg/dds/test/dap/integration/dart_test_test.dart
index 263aa9c..3d15b82 100644
--- a/pkg/dds/test/dap/integration/dart_test_test.dart
+++ b/pkg/dds/test/dap/integration/dart_test_test.dart
@@ -298,7 +298,8 @@
final outputEvents = await client.collectTestOutput(
launch: () => client.attach(
vmServiceUri: 'ws://bogus.local/',
- autoResume: false,
+ autoResumeOnEntry: false,
+ autoResumeOnExit: false,
),
);
diff --git a/pkg/dds/test/dap/integration/debug_attach_test.dart b/pkg/dds/test/dap/integration/debug_attach_test.dart
index b378dd8..e3e2dcc 100644
--- a/pkg/dds/test/dap/integration/debug_attach_test.dart
+++ b/pkg/dds/test/dap/integration/debug_attach_test.dart
@@ -25,13 +25,15 @@
testFile.path,
args,
cwd: dap.testAppDir.path,
+ pauseOnExit: true, // To ensure we capture all output
);
final vmServiceUri = await waitForStdoutVmServiceBanner(proc);
final outputEvents = await dap.client.collectOutput(
launch: () => dap.client.attach(
vmServiceUri: vmServiceUri.toString(),
- autoResume: true,
+ autoResumeOnEntry: true,
+ autoResumeOnExit: true,
cwd: dap.testAppDir.path,
),
);
@@ -78,11 +80,13 @@
['one', 'two'],
cwd: dap.testAppDir.path,
vmArgs: ['--write-service-info=${Uri.file(vmServiceInfoFilePath)}'],
+ pauseOnExit: true, // To ensure we capture all output
);
final outputEvents = await dap.client.collectOutput(
launch: () => dap.client.attach(
vmServiceInfoFile: vmServiceInfoFilePath,
- autoResume: true,
+ autoResumeOnEntry: true,
+ autoResumeOnExit: true,
cwd: dap.testAppDir.path,
),
);
@@ -125,6 +129,11 @@
testFile.path,
[],
cwd: dap.testAppDir.path,
+ // Disable user-pause-on-exit because we're checking DAP resumes and
+ // if the VM waits for user-resume, we won't complete. We don't want to
+ // send an explicit user-resume because that would force resume,
+ // invalidating this test that we did a DAP resume.
+ pauseOnExit: false,
);
final vmServiceUri = await waitForStdoutVmServiceBanner(proc);
@@ -135,7 +144,8 @@
client.start(
launch: () => client.attach(
vmServiceUri: vmServiceUri.toString(),
- autoResume: false,
+ autoResumeOnEntry: false,
+ autoResumeOnExit: false,
cwd: dap.testAppDir.path,
),
),
diff --git a/pkg/dds/test/dap/integration/debug_breakpoints_test.dart b/pkg/dds/test/dap/integration/debug_breakpoints_test.dart
index 6e4c49e..5207121 100644
--- a/pkg/dds/test/dap/integration/debug_breakpoints_test.dart
+++ b/pkg/dds/test/dap/integration/debug_breakpoints_test.dart
@@ -646,7 +646,8 @@
client2.start(
launch: () => client2.attach(
vmServiceUri: vmServiceUri.toString(),
- autoResume: false,
+ autoResumeOnEntry: false,
+ autoResumeOnExit: false,
cwd: dap.testAppDir.path,
),
),
diff --git a/pkg/dds/test/dap/integration/debug_test.dart b/pkg/dds/test/dap/integration/debug_test.dart
index 33f3ea9..3167b5b 100644
--- a/pkg/dds/test/dap/integration/debug_test.dart
+++ b/pkg/dds/test/dap/integration/debug_test.dart
@@ -161,7 +161,7 @@
final testFile = dap.createTestFile(simpleArgPrintingProgram);
// Run the script, expecting a Stopped event.
- final stop = dap.client.expectStop('pause');
+ final stop = dap.client.expectStop('exit');
await Future.wait([
stop,
dap.client.initialize(),
@@ -176,6 +176,97 @@
], eagerError: true);
});
+ test('does not resume isolates if user passes --pause-isolates-on-start',
+ () async {
+ // Internally we always pass --pause-isolates-on-start and resume the
+ // isolates after setting any breakpoints.
+ //
+ // However if a user passes this flag explicitly, we should not
+ // auto-resume because they might be trying to debug something.
+ final testFile = dap.createTestFile(simpleArgPrintingProgram);
+
+ // Run the script, expecting a Stopped event.
+ final stop = dap.client.expectStop('entry');
+ await Future.wait([
+ stop,
+ dap.client.initialize(),
+ dap.client.launch(
+ testFile.path,
+ toolArgs: ["--pause-isolates-on-start"],
+ ),
+ ], eagerError: true);
+
+ // Resume and expect termination.
+ await Future.wait([
+ dap.client.event('terminated'),
+ dap.client.continue_((await stop).threadId!),
+ ], eagerError: true);
+ });
+
+ test('receives thread, stopped, continued events during pause/resume',
+ () async {
+ final client = dap.client;
+ final testFile = dap.createTestFile(debuggerPauseAndPrintManyProgram);
+
+ // Collect interesting events that we want to verify exist and in the
+ // right order.
+ final interestingEvents = const {
+ 'thread',
+ 'stopped',
+ 'continued',
+ 'terminated'
+ };
+ final eventsFuture = client.allEvents
+ .where((e) => interestingEvents.contains(e.event))
+ .map((e) {
+ // Map onto a descriptive string for verifying later.
+ final reason = (e.body as Map<String, Object?>)['reason'] as String?;
+ return reason != null ? '${e.event} ($reason)' : e.event;
+ }).toList();
+
+ // Start the program and wait to pause on `debugger()`.
+ final stoppedFuture = client.expectStop('step');
+ await client.start(file: testFile);
+ final threadId = (await stoppedFuture).threadId!;
+
+ // Step 3 times and wait for the corresponding stop.
+ for (var i = 0; i < 3; i++) {
+ client.next(threadId);
+ await client.stoppedEvents.first;
+ }
+
+ // Resume to run to end.
+ client.continue_(threadId);
+
+ // Verify we had the expected events.
+ expect(
+ await eventsFuture,
+ [
+ 'thread (started)',
+ 'stopped (entry)',
+ 'continued',
+ // stop on debugger()
+ 'stopped (step)',
+ // step 1
+ 'continued',
+ 'stopped (step)',
+ // step 2
+ 'continued',
+ 'stopped (step)',
+ // step 3
+ 'continued',
+ 'stopped (step)',
+ // continue
+ 'continued',
+ // pause-on-exit to drain stdout and handle looking up URIs
+ 'stopped (exit)',
+ // finished
+ 'thread (exited)',
+ 'terminated',
+ ],
+ );
+ });
+
for (final outputKind in ['stdout', 'stderr']) {
test('sends $outputKind output events in the correct order', () async {
// Output events that have their URIs mapped will be processed slowly due
@@ -760,18 +851,21 @@
}, timeout: Timeout.none);
group('debug mode', () {
- test('can run without DDS', () async {
- final dap = await DapTestSession.setUp(additionalArgs: ['--no-dds']);
- addTearDown(dap.tearDown);
+ test(
+ 'can run without DDS',
+ () async {
+ final dap = await DapTestSession.setUp(additionalArgs: ['--no-dds']);
+ addTearDown(dap.tearDown);
- final client = dap.client;
- final testFile = dap.createTestFile(simpleBreakpointProgram);
- final breakpointLine = lineWith(testFile, breakpointMarker);
+ final client = dap.client;
+ final testFile = dap.createTestFile(simpleBreakpointProgram);
+ final breakpointLine = lineWith(testFile, breakpointMarker);
- await client.hitBreakpoint(testFile, breakpointLine);
+ await client.hitBreakpoint(testFile, breakpointLine);
- expect(await client.ddsAvailable, isFalse);
- });
+ expect(await client.ddsAvailable, isFalse);
+ },
+ );
test('can run without auth codes', () async {
final dap =
diff --git a/pkg/dds/test/dap/integration/test_client.dart b/pkg/dds/test/dap/integration/test_client.dart
index 936a6d3..1a09111 100644
--- a/pkg/dds/test/dap/integration/test_client.dart
+++ b/pkg/dds/test/dap/integration/test_client.dart
@@ -135,7 +135,8 @@
/// Send an attachRequest to the server, asking it to attach to an existing
/// Dart program.
Future<Response> attach({
- required bool autoResume,
+ required bool autoResumeOnEntry,
+ required bool autoResumeOnExit,
String? vmServiceUri,
String? vmServiceInfoFile,
String? cwd,
@@ -154,10 +155,19 @@
// When attaching, the paused VM will not be automatically unpaused, but
// instead send a Stopped(reason: 'entry') event. Respond to this by
// resuming (if requested).
- final resumeFuture = autoResume
+ final resumeFuture = autoResumeOnEntry
? expectStop('entry').then((event) => continue_(event.threadId!))
: null;
+ // Also handle resuming on exit. This should be done if the test has
+ // started the app with a user-provided pause-on-exit but wants to
+ // simulate the user resuming after the exit pause.
+ if (autoResumeOnExit) {
+ stoppedEvents
+ .firstWhere((e) => e.reason == 'exit')
+ .then((event) => continue_(event.threadId!));
+ }
+
cwd ??= defaultCwd;
final attachResponse = sendRequest(
DartAttachRequestArguments(
@@ -227,13 +237,22 @@
/// Returns a Future that completes with the next [event] event.
Future<Event> event(String event) => _logIfSlow(
'Event "$event"',
- _eventController.stream.firstWhere((e) => e.event == event,
+ allEvents.firstWhere((e) => e.event == event,
orElse: () =>
throw 'Did not receive $event event before stream closed'));
/// Returns a stream for [event] events.
Stream<Event> events(String event) {
- return _eventController.stream.where((e) => e.event == event);
+ return allEvents.where((e) => e.event == event);
+ }
+
+ Stream<Event> get allEvents => _eventController.stream;
+
+ /// Returns a stream for 'stopped' events.
+ Stream<StoppedEventBody> get stoppedEvents {
+ return allEvents
+ .where((e) => e.event == 'stopped')
+ .map((e) => StoppedEventBody.fromJson(e.body as Map<String, Object?>));
}
/// Returns a stream for standard progress events.
@@ -243,8 +262,7 @@
'progressUpdate',
'progressEnd'
};
- return _eventController.stream
- .where((e) => standardProgressEvents.contains(e.event));
+ return allEvents.where((e) => standardProgressEvents.contains(e.event));
}
/// Returns a stream for custom Dart progress events.
@@ -254,8 +272,7 @@
'dart.progressUpdate',
'dart.progressEnd'
};
- return _eventController.stream
- .where((e) => customProgressEvents.contains(e.event));
+ return allEvents.where((e) => customProgressEvents.contains(e.event));
}
/// Records a handler for when the server sends a [request] request.
@@ -902,14 +919,26 @@
///
/// If [file] or [line] are provided, they will be checked against the stop
/// location for the top stack frame.
+ ///
+ /// Stopped-on-entry events will be automatically skipped unless
+ /// [skipFirstStopOnEntry] is `false` or [reason] is `"entry"`.
Future<StoppedEventBody> expectStop(
String reason, {
File? file,
int? line,
String? sourceName,
+ bool? skipFirstStopOnEntry,
}) async {
- final e = await event('stopped');
- final stop = StoppedEventBody.fromJson(e.body as Map<String, Object?>);
+ skipFirstStopOnEntry ??= reason != 'entry';
+ assert(skipFirstStopOnEntry != (reason == 'entry'));
+
+ // Unless we're specifically waiting for stop-on-entry, skip over those
+ // events because they can be emitted at during startup because now we use
+ // readyToResume we don't know if the isolate will be immediately unpaused
+ // and the client needs to have a consistent view of threads.
+ final stop = skipFirstStopOnEntry
+ ? await stoppedEvents.skipWhile((e) => e.reason == 'entry').first
+ : await stoppedEvents.first;
expect(stop.reason, equals(reason));
final result =
diff --git a/pkg/dds/test/dap/integration/test_scripts.dart b/pkg/dds/test/dap/integration/test_scripts.dart
index dc32ca8..ed265669 100644
--- a/pkg/dds/test/dap/integration/test_scripts.dart
+++ b/pkg/dds/test/dap/integration/test_scripts.dart
@@ -227,6 +227,23 @@
}
''';
+/// A program that immediately pauses with debugger() and then has 5 print
+/// statements.
+///
+/// Used for verifying breaking and stepping behaviour.
+const debuggerPauseAndPrintManyProgram = '''
+ import 'dart:developer';
+
+ void main(List<String> args) async {
+ debugger();
+ print('1');
+ print('2');
+ print('3');
+ print('4');
+ print('5');
+ }
+''';
+
final simpleBreakpointProgramWith50ExtraLines = '''
void main(List<String> args) async {
print('Hello!'); $breakpointMarker
diff --git a/pkg/dds/test/dap/integration/test_support.dart b/pkg/dds/test/dap/integration/test_support.dart
index 031cbcd..15ba49c 100644
--- a/pkg/dds/test/dap/integration/test_support.dart
+++ b/pkg/dds/test/dap/integration/test_support.dart
@@ -88,6 +88,7 @@
List<String> args, {
required String cwd,
List<String>? vmArgs,
+ required bool pauseOnExit,
}) async {
final vmPath = Platform.resolvedExecutable;
vmArgs ??= [];
@@ -95,7 +96,7 @@
'--enable-vm-service=0',
'--pause_isolates_on_start',
// Use pause-on-exit so we don't lose async output events in attach tests.
- '--pause_isolates_on_exit',
+ if (pauseOnExit) '--pause_isolates_on_exit',
]);
final processArgs = [
...vmArgs,
