[beta] Revert "[dap] Simplify URI handling in IsolateManager.addBreakpoint" and "[dds/dap] Add/remove breakpoints as required instead of replacing the whole set"
This is a cherry-pick of a recent commit that reverted two DAP changes because they resulted in leaking of breakpoints/scripts in the VM during reloads.
Issue description: Leaking objects in the VM during hot reloads
What is the fix: Revert the changes that triggered this issue
Why cherry-pick: To avoid this (newly-introduced) leak
Risk: Low, this is a simple revert putting code back to how it was previously for many years
Issue Link(s): See discussion in https://docs.google.com/document/d/1eeEHG3EL4D5EbB3YiAO5X2TcCRIciFMSPKL_hFGP0Gk/edit?disco=AAABk-csO5o
Original revert CL description:
This reverts two commits (in separate patch sets in the CL):
- commit 482a7caed773bb8e6655808db4384ac90d76d89b: [dap] Simplify URI handling in IsolateManager.addBreakpoint
- commit d0a7ef44595fc7ab2991c391c6cd0609b7688402: [dds/dap] Add/remove breakpoints as required instead of replacing the whole set
The first revert is to avoid conflicts while reverting the second, and will be reapplied later (likely after the release branch, since it is not critical to include). The second revert is because this change resulted in leaked Script objects in the VM.
There is an additional change (patch set 3) to fix up the changelog/versions so they don't go backwards.
Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/440162
Change-Id: Idafb16b4d881cc472e8738e48f8e3dfda097e332
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/440661
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Slava Egorov <vegorov@google.com>
diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md
index 02accf2..413ca5f 100644
--- a/pkg/dds/CHANGELOG.md
+++ b/pkg/dds/CHANGELOG.md
@@ -1,3 +1,6 @@
+# 5.0.5
+- [DAP] The change in DDS 5.0.4 to individually add/remove breakpoints has been reverted and may be restored in a future version.
+
# 5.0.4
- [DAP] Breakpoints are now added/removed individually instead of all being cleared and re-added during a `setBreakpoints` request. This improves performance and can avoid breakpoints flickering between unresolved/resolved when adding new breakpoints in the same file.
diff --git a/pkg/dds/lib/src/dap/adapters/dart.dart b/pkg/dds/lib/src/dap/adapters/dart.dart
index a27d865..5909aa1 100644
--- a/pkg/dds/lib/src/dap/adapters/dart.dart
+++ b/pkg/dds/lib/src/dap/adapters/dart.dart
@@ -1577,70 +1577,26 @@
: name!;
// Use a completer to track when the response is sent, so any events related
- // to new breakpoints will not be sent to the client before the response
- // here which provides the IDs to the client.
+ // to these breakpoints are not sent before the client has the IDs.
final completer = Completer<void>();
- // Map the provided breakpoints onto either new or existing instances of
- // [ClientBreakpoint] that we use to track the clients breakpoints
- // internally.
- final clientBreakpoints = breakpoints.map((bp) {
- return
- // First try to match an existing breakpoint so we can avoid deleting
- // and re-creating all breakpoints if a new one is added to a file.
- isolateManager.findExistingClientBreakpoint(uri, bp) ??
- ClientBreakpoint(bp, completer.future);
- }).toList();
+ final clientBreakpoints = breakpoints
+ .map((bp) => ClientBreakpoint(bp, completer.future))
+ .toList();
+ await isolateManager.setBreakpoints(uri, clientBreakpoints);
- // Any breakpoints that are not in our new set will need to be removed from
- // the VM.
- //
- // Because multiple client breakpoints may resolve to the same VM breakpoint
- // we must exclude any that still remain in one of the kept breakpoints.
- final referencedVmBreakpoints =
- clientBreakpoints.map((bp) => bp.forThread.values).toSet();
- final breakpointsToRemove = isolateManager.clientBreakpointsByUri[uri]
- ?.toSet()
- // Remove any we're reusing.
- .difference(clientBreakpoints.toSet())
- // Remove any that map to VM breakpoints that are still referenced
- // because we'll want to keep them.
- .where((clientBreakpoint) => clientBreakpoint.forThread.values
- .none(referencedVmBreakpoints.contains));
-
- // Store this new set of breakpoints as the current set for this URI.
- isolateManager.recordLatestClientBreakpoints(uri, clientBreakpoints);
-
- // Prepare the response with the existing values before we start updating.
- final breakpointResponse = SetBreakpointsResponseBody(
+ sendResponse(SetBreakpointsResponseBody(
breakpoints: clientBreakpoints
+ // Send breakpoints back as unverified and with our generated IDs so we
+ // can update them with a 'breakpoint' event when we get the
+ // 'BreakpointAdded'/'BreakpointResolved' events from the VM.
.map((bp) => Breakpoint(
id: bp.id,
- verified: bp.verified,
- line: bp.verified ? bp.resolvedLine : null,
- column: bp.verified ? bp.resolvedColumn : null,
- message: bp.verified ? null : bp.verifiedMessage,
- reason: bp.verified ? null : bp.verifiedReason))
+ verified: false,
+ message: 'Breakpoint has not yet been resolved',
+ reason: 'pending'))
.toList(),
- );
-
- // Update the breakpoints for all existing threads.
- await Future.wait(isolateManager.threads.map((thread) async {
- // Remove the deleted breakpoints.
- if (breakpointsToRemove != null) {
- await Future.wait(breakpointsToRemove.map((clientBreakpoint) =>
- isolateManager.removeBreakpoint(clientBreakpoint, thread)));
- }
-
- // Add the new breakpoints.
- await Future.wait(clientBreakpoints.map((clientBreakpoint) async {
- if (!clientBreakpoint.isKnownToVm) {
- await isolateManager.addBreakpoint(clientBreakpoint, thread, uri);
- }
- }));
- }));
-
- sendResponse(breakpointResponse);
+ ));
completer.complete();
}
diff --git a/pkg/dds/lib/src/dap/isolate_manager.dart b/pkg/dds/lib/src/dap/isolate_manager.dart
index d135634..bfc2f49 100644
--- a/pkg/dds/lib/src/dap/isolate_manager.dart
+++ b/pkg/dds/lib/src/dap/isolate_manager.dart
@@ -63,9 +63,9 @@
/// [debugExternalPackageLibraries] in one step.
bool debugExternalPackageLibraries = true;
- /// Tracks breakpoints last provided by the client (by the client URI) so they
- /// can be sent to new isolates that appear after initial breakpoints were sent.
- final Map<String, List<ClientBreakpoint>> clientBreakpointsByUri = {};
+ /// 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 = {};
/// Tracks client breakpoints by the ID assigned by the VM so we can look up
/// conditions/logpoints when hitting breakpoints.
@@ -89,11 +89,22 @@
/// breakpoint).
///
/// When new breakpoints are added by the client, we must check this map to
- /// see it's an already-resolved breakpoint so that we can send resolution
+ /// see it's al already-resolved breakpoint so that we can send resolution
/// info to the client.
final Map<_UniqueVmBreakpointId, vm.Event> _breakpointResolvedEventsByVmId =
{};
+ /// Tracks breakpoints created in the VM so they can be removed when the
+ /// editor sends new breakpoints (currently the editor just sends a new list
+ /// and not requests to add/remove).
+ ///
+ /// Breakpoints are indexed by their ID so that duplicates are not stored even
+ /// if multiple client breakpoints resolve to a single VM breakpoint.
+ ///
+ /// IsolateId -> Uri -> breakpointId -> VM Breakpoint.
+ final Map<String, Map<String, Map<String, vm.Breakpoint>>>
+ _vmBreakpointsByIsolateIdAndUri = {};
+
/// The exception pause mode last provided by the client.
///
/// This will be sent to isolates as they are created, and to all existing
@@ -496,27 +507,32 @@
));
}
- // Tracks the latest set of breakpoints sent by the client that should
- // be sent to any new isolates that start.
- void recordLatestClientBreakpoints(
+ /// Records breakpoints for [uri].
+ ///
+ /// [breakpoints] represents the new set and entirely replaces anything given
+ /// before.
+ Future<void> setBreakpoints(
String uri,
List<ClientBreakpoint> breakpoints,
- ) {
- clientBreakpointsByUri[uri] = breakpoints;
+ ) async {
+ // Track the breakpoints to get sent to any new isolates that start.
+ _clientBreakpointsByUri[uri] = breakpoints;
+
+ // Send the breakpoints to all existing threads.
+ await Future.wait(_threadsByThreadId.values
+ .map((thread) => _sendBreakpoints(thread, uri: uri)));
}
/// Clears all breakpoints.
Future<void> clearAllBreakpoints() async {
- // Group all breakpoints for all URIs before clearing the list.
- final clientBreakpointsToDelete =
- clientBreakpointsByUri.values.expand((bps) => bps).toList();
- clientBreakpointsByUri.clear();
+ // Clear all breakpoints for each URI. Do not remove the items from the map
+ // as that will stop them being tracked/sent by the call below.
+ _clientBreakpointsByUri.updateAll((key, value) => []);
- // Remove all breakpoints for all threads.
- await Future.wait(clientBreakpointsToDelete.map((clientBreakpoint) {
- return Future.wait(_threadsByThreadId.values
- .map((thread) => removeBreakpoint(clientBreakpoint, thread)));
- }));
+ // Send the breakpoints to all existing threads.
+ await Future.wait(
+ _threadsByThreadId.values.map((thread) => _sendBreakpoints(thread)),
+ );
}
/// Records exception pause mode as one of 'None', 'Unhandled' or 'All'. All
@@ -576,12 +592,7 @@
_sendExceptionPauseMode(thread),
], eagerError: true);
- await Future.wait(clientBreakpointsByUri.entries.map((mapEntry) async {
- var clientUri = mapEntry.key;
- var clientBreakpoints = mapEntry.value;
- await Future.wait(clientBreakpoints.map((clientBreakpoint) =>
- addBreakpoint(clientBreakpoint, thread, clientUri)));
- }));
+ await _sendBreakpoints(thread);
} 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
@@ -879,19 +890,12 @@
// This is always resolved because of the check above.
final location = breakpoint.location;
final resolvedLocation = location as vm.SourceLocation;
-
- // Track that this breakpoint has been resolved, so that if we are asked to
- // set the same breakpoint in future, we can immediately return
- // resolved=true in the response.
- clientBreakpoint.resolved(resolvedLocation.line, resolvedLocation.column);
-
final updatedBreakpoint = Breakpoint(
id: clientBreakpoint.id,
- line: clientBreakpoint.resolvedLine,
- column: clientBreakpoint.resolvedColumn,
- verified: clientBreakpoint.verified,
+ line: resolvedLocation.line,
+ column: resolvedLocation.column,
+ verified: true,
);
-
// Ensure we don't send the breakpoint event until the client has been
// given the breakpoint ID by queueing it.
clientBreakpoint.queueAction(
@@ -928,18 +932,11 @@
userMessage = terseMessageMatch.group(1) ?? userMessage;
}
- // Record the failure state only if we don't already have a success state
- // because if we set a breakpoint in some isolates but not others, we do not
- // want to show that as unresolved.
- if (!clientBreakpoint.verified) {
- clientBreakpoint.unresolved(reason: 'failed', message: userMessage);
- }
-
final updatedBreakpoint = Breakpoint(
id: clientBreakpoint.id,
- verified: clientBreakpoint.verified,
- message: clientBreakpoint.verifiedMessage,
- reason: clientBreakpoint.verifiedReason,
+ verified: false,
+ message: userMessage,
+ reason: 'failed',
);
// Ensure we don't send the breakpoint event until the client has been
// given the breakpoint ID by queueing it.
@@ -1042,58 +1039,13 @@
await service.reloadSources(isolateId);
}
- /// Tries to find an existing [ClientBreakpoint] matching the location of the
- /// provided [breakpoint] so that it can be reused/kept when updating
- /// breakpoints.
- ClientBreakpoint? findExistingClientBreakpoint(
- String clientUri, SourceBreakpoint breakpoint) {
- var clientBreakpoints = clientBreakpointsByUri[clientUri];
- if (clientBreakpoints == null) {
- return null;
- }
-
- return clientBreakpoints.firstWhereOrNull((clientBreakpoint) =>
- // These conditions must cover all fields that would be sent to the VM.
- // They do not need to include things like `condition` which we check
- // DAP-side.
-
- // We always compare breakpoints based on the original location and not
- // the resolved location, because clients will not update the underlying
- // breakpoints with resolution, they will only assign a temporary
- // overriden location.
- //
- // See https://github.com/microsoft/vscode/issues/250453.
- clientBreakpoint.breakpoint.line == breakpoint.line &&
- clientBreakpoint.breakpoint.column == breakpoint.column);
- }
-
- /// Converts local Google3 or SDK file paths to URIs VM can recognize.
+ /// Sets breakpoints for an individual isolate.
///
- /// ```
- /// sdk-path/lib/core/print.dart -> org-dartlang-sdk://sdk/lib/core/print.dart
- /// google/*/google3/<path> -> google3://<path>
- /// ```
- ///
- /// VM is capable of setting breakpoints using both original (`package`
- /// scheme) URIs or their resolved variants - which means no convertion
- /// is necessary if local path is the same as the resolved path known to the
- /// VM.
- ///
- /// Google3 paths and Dart SDK paths however require special handling:
- /// because in both cases resolved paths we given using *multi-root
- /// filesystem* accessed via a special scheme (`google3` and
- /// `org-dartlang-sdk` respectively) to hide local file layout from the
- /// front-end.
- Uri _fixSDKOrGoogle3Paths(Uri sourcePathUri) {
- return _adapter.convertUriToOrgDartlangSdk(sourcePathUri) ??
- _convertPathToGoogle3Uri(sourcePathUri) ??
- sourcePathUri;
- }
-
- /// Creates a breakpoint in [clientUri] for [thread] in the VM that
- /// corresponds to [clientBreakpoint] received from the client.
- Future<void> addBreakpoint(ClientBreakpoint clientBreakpoint,
- ThreadInfo thread, String clientUri) async {
+ /// If [uri] is provided, only breakpoints for that URI will be sent (used
+ /// when breakpoints are modified for a single file in the editor). Otherwise
+ /// breakpoints for all previously set URIs will be sent (used for
+ /// newly-created isolates).
+ Future<void> _sendBreakpoints(ThreadInfo thread, {String? uri}) async {
final service = _adapter.vmService;
if (!debug || service == null) {
return;
@@ -1101,63 +1053,78 @@
final isolateId = thread.isolate.id!;
- try {
- // Some file URIs (like SDK sources) need to be converted to
- // appropriate internal URIs to be able to set breakpoints.
- final vmUri = _fixSDKOrGoogle3Paths(Uri.parse(clientUri));
- final vmBp = await service.addBreakpointWithScriptUri(
- isolateId, vmUri.toString(), clientBreakpoint.breakpoint.line,
- column: clientBreakpoint.breakpoint.column);
- clientBreakpoint.forThread[thread] = vmBp;
- final uniqueBreakpointId = (isolateId: isolateId, breakpointId: vmBp.id!);
+ // If we were passed a single URI, we should send breakpoints only for that
+ // (this means the request came from the client), otherwise we should send
+ // all of them (because this is a new/restarting isolate).
+ final uris = uri != null ? [uri] : _clientBreakpointsByUri.keys.toList();
- // Store this client breakpoint by the VM ID, so when we get events
- // from the VM we can map them back to client breakpoints (for example
- // to send resolved events).
- _clientBreakpointsByVmId
- .putIfAbsent(uniqueBreakpointId, () => [])
- .add(clientBreakpoint);
+ for (final uri in uris) {
+ // Clear existing breakpoints.
+ final existingBreakpointsForIsolate =
+ _vmBreakpointsByIsolateIdAndUri.putIfAbsent(isolateId, () => {});
+ final existingBreakpointsForIsolateAndUri =
+ existingBreakpointsForIsolate.putIfAbsent(uri, () => {});
+ // Before doing async work, take a copy of the breakpoints to remove
+ // and remove them from the list, so any subsequent calls here don't
+ // try to remove the same ones multiple times.
+ final breakpointsToRemove =
+ existingBreakpointsForIsolateAndUri.values.toList();
+ existingBreakpointsForIsolateAndUri.clear();
+ await Future.forEach<vm.Breakpoint>(breakpointsToRemove, (bp) async {
+ try {
+ await service.removeBreakpoint(isolateId, bp.id!);
+ } catch (e) {
+ // Swallow errors removing breakpoints rather than failing the whole
+ // request as it's very possible that an isolate exited while we were
+ // sending this and the request will fail.
+ _adapter.logger?.call('Failed to remove old breakpoint $e');
+ }
+ });
- // Queue any resolved events that may have already arrived
- // (either because the VM sent them before responding to us, or
- // because it gave us an existing VM breakpoint because it resolved to
- // the same location as another).
- final resolvedEvent = _breakpointResolvedEventsByVmId[uniqueBreakpointId];
- if (resolvedEvent != null) {
- queueBreakpointResolutionEvent(resolvedEvent, clientBreakpoint);
- }
- } catch (e) {
- // Swallow errors setting breakpoints rather than failing the whole
- // request as it's very easy for editors to send us breakpoints that
- // aren't valid any more.
- _adapter.logger?.call('Failed to add breakpoint $e');
- queueFailedBreakpointEvent(e, clientBreakpoint);
- }
- }
+ // Set new breakpoints.
+ final newBreakpoints = _clientBreakpointsByUri[uri] ?? const [];
+ await Future.forEach<ClientBreakpoint>(newBreakpoints, (bp) async {
+ try {
+ // Some file URIs (like SDK sources) need to be converted to
+ // appropriate internal URIs to be able to set breakpoints.
+ final vmUri = await thread.resolvePathToUri(Uri.parse(uri));
- /// Removes [clientBreakpoint] from [thread] in the VM.
- Future<void> removeBreakpoint(
- ClientBreakpoint clientBreakpoint, ThreadInfo thread) async {
- final service = _adapter.vmService;
- if (!debug || service == null) {
- return;
- }
+ if (vmUri == null) {
+ return;
+ }
- final isolateId = thread.isolate.id!;
- final vmBreakpoint = clientBreakpoint.forThread[thread];
- if (vmBreakpoint == null) {
- // This isolate didn't have this breakpoint.
- return;
- }
+ final vmBp = await service.addBreakpointWithScriptUri(
+ isolateId, vmUri.toString(), bp.breakpoint.line,
+ column: bp.breakpoint.column);
+ final vmBpId = vmBp.id!;
+ final uniqueBreakpointId =
+ (isolateId: isolateId, breakpointId: vmBp.id!);
+ existingBreakpointsForIsolateAndUri[vmBpId] = vmBp;
- try {
- await service.removeBreakpoint(isolateId, vmBreakpoint.id!);
- clientBreakpoint.forThread.remove(thread);
- } catch (e) {
- // Swallow errors removing breakpoints rather than failing the whole
- // request as it's very possible that an isolate exited while we were
- // sending this and the request will fail.
- _adapter.logger?.call('Failed to remove old breakpoint $e');
+ // Store this client breakpoint by the VM ID, so when we get events
+ // from the VM we can map them back to client breakpoints (for example
+ // to send resolved events).
+ _clientBreakpointsByVmId
+ .putIfAbsent(uniqueBreakpointId, () => [])
+ .add(bp);
+
+ // Queue any resolved events that may have already arrived
+ // (either because the VM sent them before responding to us, or
+ // because it gave us an existing VM breakpoint because it resolved to
+ // the same location as another).
+ final resolvedEvent =
+ _breakpointResolvedEventsByVmId[uniqueBreakpointId];
+ if (resolvedEvent != null) {
+ queueBreakpointResolutionEvent(resolvedEvent, bp);
+ }
+ } catch (e) {
+ // Swallow errors setting breakpoints rather than failing the whole
+ // request as it's very easy for editors to send us breakpoints that
+ // aren't valid any more.
+ _adapter.logger?.call('Failed to add breakpoint $e');
+ queueFailedBreakpointEvent(e, bp);
+ }
+ });
}
}
@@ -1403,32 +1370,49 @@
return _currentEvaluationZoneId;
}
- /// Resolves a source file URI into a original `package://` or SDK URI.
+ /// Resolves a source file path (or URI) into a URI for the VM.
///
- /// ```
- /// sdk-path/lib/core/print.dart -> org-dartlang-sdk://sdk/lib/core/print.dart
+ /// sdk-path/lib/core/print.dart -> dart:core/print.dart
/// c:\foo\bar -> package:foo/bar
- /// ```
+ /// dart-macro+file:///c:/foo/bar -> dart-macro+package:foo/bar
///
- /// This helper is used when trying to find [vm.Script] by matching its
- /// `uri`.
- Future<Uri?> _convertToPackageOrSdkPath(Uri sourcePathUri) async {
- final uri = _manager._fixSDKOrGoogle3Paths(sourcePathUri);
- if (uri.isScheme('org-dartlang-sdk')) {
- return uri; // No package path exists for SDK sources.
+ /// This is required so that when the user sets a breakpoint in an SDK source
+ /// (which they may have navigated to via the Analysis Server) we generate a
+ /// valid URI that the VM would create a breakpoint for.
+ ///
+ /// Because the VM supports using `file:` URIs in many places, we usually do
+ /// not need to convert file paths into `package:` URIs, however this will
+ /// be done if [forceResolveFileUris] is `true`.
+ Future<Uri?> resolvePathToUri(
+ Uri sourcePathUri, {
+ bool forceResolveFileUris = false,
+ }) async {
+ final sdkUri = _manager._adapter.convertUriToOrgDartlangSdk(sourcePathUri);
+ if (sdkUri != null) {
+ return sdkUri;
}
+ final google3Uri = _convertPathToGoogle3Uri(sourcePathUri);
+ final uri = google3Uri ?? sourcePathUri;
+
+ // As an optimisation, we don't resolve file -> package URIs in many cases
+ // because the VM can set breakpoints for file: URIs anyway. However for
+ // G3 or if [forceResolveFileUris] is set, we will.
+ final performResolve = google3Uri != null || forceResolveFileUris;
+
// TODO(dantup): Consider caching results for this like we do for
// resolveUriToPath (and then forceResolveFileUris can be removed and just
// always used).
- final packageUriList = await _manager._adapter.vmService
- ?.lookupPackageUris(isolate.id!, [uri.toString()]);
+ final packageUriList = performResolve
+ ? await _manager._adapter.vmService
+ ?.lookupPackageUris(isolate.id!, [uri.toString()])
+ : null;
final packageUriString = packageUriList?.uris?.firstOrNull;
if (packageUriString != null) {
// Use package URI if we resolved something
return Uri.parse(packageUriString);
- } else if (uri.isScheme('google3')) {
+ } else if (google3Uri != null) {
// If we failed to resolve and was a Google3 URI, return null
return null;
} else {
@@ -1615,6 +1599,28 @@
/// that are round-tripped to the client.
int storeData(Object data) => _manager.storeData(this, data);
+ Uri? _convertPathToGoogle3Uri(Uri input) {
+ // TODO(dantup): Do we need to handle non-file here? Eg. can we have
+ // dart-macro+file:/// for a google3 path?
+ if (!input.isScheme('file')) {
+ return null;
+ }
+ final inputPath = input.toFilePath();
+
+ const search = '/google3/';
+ if (inputPath.startsWith('/google') && inputPath.contains(search)) {
+ var idx = inputPath.indexOf(search);
+ var remainingPath = inputPath.substring(idx + search.length);
+ return Uri(
+ scheme: 'google3',
+ host: '',
+ path: remainingPath,
+ );
+ }
+
+ return null;
+ }
+
/// Converts a VM-returned URI to a file-like URI, taking org-dartlang-sdk
/// schemes into account.
///
@@ -1700,9 +1706,12 @@
Future<vm.LibraryRef?> getLibraryForFileUri(Uri scriptFileUri) async {
// We start with a file URI and need to find the Library (via the script).
//
- // We need to handle mismatched drive letters, and also file vs package
+ // We need to handle msimatched drive letters, and also file vs package
// URIs.
- final scriptResolvedUri = await _convertToPackageOrSdkPath(scriptFileUri);
+ final scriptResolvedUri = await resolvePathToUri(
+ scriptFileUri,
+ forceResolveFileUris: true,
+ );
final candidateUris = {
scriptFileUri.toString(),
normalizeUri(scriptFileUri).toString(),
@@ -1740,45 +1749,6 @@
final SourceBreakpoint breakpoint;
final int id;
- /// Whether this breakpoint has been sent to the VM (and not removed).
- bool get isKnownToVm => forThread.isNotEmpty;
-
- /// A map of [ThreadInfo] -> [vm.Breakpoint] recording the VM breakpoint for
- /// each isolate for this client breakpoint.
- ///
- /// Note: It's possible for multiple [ClientBreakpoint]s to resolve to the
- /// same VM breakpoint!
- Map<ThreadInfo, vm.Breakpoint> forThread = {};
-
- /// Whether this breakpoint was previously been verified.
- bool get verified => _verified;
-
- bool _verified = false;
-
- /// A user-friendly explanation of why this breakpoint could not be resolved
- /// (if [verified] is `false`, otherwise `null`).
- String? get verifiedMessage => _verifiedMessage;
-
- String? _verifiedMessage = 'Breakpoint has not yet been resolved';
-
- /// A machine-readable reason (specified by DAP) of why this breakpoint is not
- /// resolved (if [verified] is `false`, otherwise `null`).
- String? get verifiedReason => _verifiedReason;
-
- String? _verifiedReason = 'pending';
-
- /// The line that this breakpoint resolved to, or the request line if not
- /// resolved.
- int get resolvedLine => _resolvedLine ?? breakpoint.line;
-
- int? _resolvedLine;
-
- /// The column that this breakpoint resolved to, or the request column if not
- /// resolved.
- int? get resolvedColumn => _resolvedColumn ?? breakpoint.column;
-
- int? _resolvedColumn;
-
/// A [Future] that completes with the last action that sends breakpoint
/// information to the client, to ensure breakpoint events are always sent
/// in-order and after the initial response sending the IDs to the client.
@@ -1795,22 +1765,6 @@
_lastActionFuture = actionFuture;
return actionFuture;
}
-
- /// Marks that this breakpoint was resolved.
- void resolved(int? line, int? column) {
- _verified = true;
- _verifiedReason = null;
- _verifiedMessage = null;
- _resolvedLine = line;
- _resolvedColumn = column;
- }
-
- /// Marks that this breakpoint was not resolved and why.
- void unresolved({required String reason, required String message}) {
- _verified = false;
- _verifiedReason = reason;
- _verifiedMessage = message;
- }
}
/// Tracks actions resulting from `BreakpointAdded`/`BreakpointResolved` events
@@ -1843,25 +1797,3 @@
StoredData(this.thread, this.data);
}
-
-Uri? _convertPathToGoogle3Uri(Uri input) {
- // TODO(dantup): Do we need to handle non-file here? Eg. can we have
- // dart-macro+file:/// for a google3 path?
- if (!input.isScheme('file')) {
- return null;
- }
- final inputPath = input.toFilePath();
-
- const search = '/google3/';
- if (inputPath.startsWith('/google') && inputPath.contains(search)) {
- var idx = inputPath.indexOf(search);
- var remainingPath = inputPath.substring(idx + search.length);
- return Uri(
- scheme: 'google3',
- host: '',
- path: remainingPath,
- );
- }
-
- return null;
-}
diff --git a/pkg/dds/pubspec.yaml b/pkg/dds/pubspec.yaml
index eb5e983..89c4c0e 100644
--- a/pkg/dds/pubspec.yaml
+++ b/pkg/dds/pubspec.yaml
@@ -1,5 +1,5 @@
name: dds
-version: 5.0.4
+version: 5.0.5
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/dap/integration/dart_test_test.dart b/pkg/dds/test/dap/integration/dart_test_test.dart
index 228d9d6..4011824 100644
--- a/pkg/dds/test/dap/integration/dart_test_test.dart
+++ b/pkg/dds/test/dap/integration/dart_test_test.dart
@@ -203,7 +203,7 @@
// Add breakpoints to the 4 lines after the current one, one at a time.
// Capture the IDs of all breakpoints added.
- final breakpointLinesToSend = <int>[];
+ final breakpointLinesToSend = <int>[breakpointLine];
final addedBreakpoints = <int>{};
for (var i = 1; i <= 4; i++) {
breakpointLinesToSend.add(breakpointLine + i);
diff --git a/pkg/dds/test/dap/integration/debug_breakpoints_test.dart b/pkg/dds/test/dap/integration/debug_breakpoints_test.dart
index 533c0ef..365e60d 100644
--- a/pkg/dds/test/dap/integration/debug_breakpoints_test.dart
+++ b/pkg/dds/test/dap/integration/debug_breakpoints_test.dart
@@ -51,7 +51,7 @@
// Add breakpoints to the 4 lines after the current one, one at a time.
// Capture the IDs of all breakpoints added.
- final breakpointLinesToSend = <int>[];
+ final breakpointLinesToSend = <int>[breakpointLine];
final addedBreakpoints = <int>{};
for (var i = 1; i <= 4; i++) {
breakpointLinesToSend.add(breakpointLine + i);
@@ -75,86 +75,6 @@
expect(resolvedBreakpoints, addedBreakpoints);
});
- testWithUriConfigurations(() => dap,
- 'does not re-resolve existing breakpoints when new ones are added',
- () async {
- final client = dap.client;
- final testFile = dap.createTestFile(simpleMultiBreakpointProgram);
- final breakpointLine = lineWith(testFile, breakpointMarker);
-
- // Start the app and hit the initial breakpoint.
- await client.hitBreakpoint(testFile, breakpointLine);
-
- // Collect any breakpoint events in a simple text format for verifying.
- final breakpointEvents = <String>[];
- final breakpointResolveSubscription =
- client.breakpointChangeEvents.listen((event) {
- var breakpoint = event.breakpoint;
- var id = breakpoint.id!;
- var verified = breakpoint.verified;
- var reason = breakpoint.reason;
- var description = verified ? 'verified' : 'not verified ($reason)';
- breakpointEvents.add('Breakpoint $id $description');
- });
-
- // Test adding breakpoints to the 4 lines after the first breakpoint, one
- // at a time. Each request contains the total set of breakpoints (so the
- // first request has one breakpoint and the last request has all 4). In
- // each response, we expect the previous breakpoints to be
- // already-verified and to not get events for them. For the last one
- // breakpoint, it will not be verified and we will then get an event.
- var breakpointLines = <int>[];
- var seenBreakpointIds = <int>{};
- for (var i = 1; i <= 4; i++) {
- breakpointEvents.clear(); // Clear any events from previous iterations.
-
- // Add an additional breakpoint on the next line.
- breakpointLines.add(breakpointLine + i);
- final response = await client.setBreakpoints(testFile, breakpointLines);
- expect(response.breakpoints, hasLength(i));
-
- // Wait up to a few seconds for a resolved events to come through.
- final testUntil =
- DateTime.now().toUtc().add(const Duration(seconds: 5));
- while (DateTime.now().toUtc().isBefore(testUntil) &&
- breakpointEvents.isEmpty) {
- await pumpEventQueue(times: 5000);
- }
-
- // Verify the results for this iteration.
- for (var j = 0; j < i; j++) {
- // j is zero-based but i is one-based
- final breakpoint = response.breakpoints[j];
- final id = breakpoint.id!;
-
- // All but the last should be verified already and have existing IDs.
- if (j == i - 1) {
- expect(seenBreakpointIds.contains(id), isFalse,
- reason:
- 'Last breakpoint (index $j) should have a new unseen ID');
- expect(breakpoint.verified, isFalse,
- reason:
- 'Last breakpoint (index $j) should not yet be verified');
- seenBreakpointIds.add(id);
- } else {
- expect(seenBreakpointIds.contains(id), isTrue,
- reason:
- 'Non-last breakpoint (index $j) should have an already-seen ID because it was reused');
- expect(breakpoint.verified, isTrue,
- reason:
- 'Non-last breakpoint (index $j) should already be verified');
- }
- }
-
- // We should have had one event for that last one to be verified (others
- // were already verified).
- expect(breakpointEvents,
- ['Breakpoint ${response.breakpoints.last.id} verified']);
- }
-
- await breakpointResolveSubscription.cancel();
- });
-
testWithUriConfigurations(
() => dap, 'provides reason for failed breakpoints', () async {
final client = dap.client;
