diff --git a/pkg/dds/CHANGELOG.md b/pkg/dds/CHANGELOG.md index 9a63d217526..d1f139e2967 100644 --- a/pkg/dds/CHANGELOG.md +++ b/pkg/dds/CHANGELOG.md @@ -1,3 +1,6 @@ +# 2.2.4 +- Fix an issue where DAP adapters could try to remove the same breakpoint multiple times. + # 2.2.3 - Internal DAP changes. diff --git a/pkg/dds/lib/src/dap/isolate_manager.dart b/pkg/dds/lib/src/dap/isolate_manager.dart index ff6fc4ab634..443d1487a4e 100644 --- a/pkg/dds/lib/src/dap/isolate_manager.dart +++ b/pkg/dds/lib/src/dap/isolate_manager.dart @@ -624,7 +624,12 @@ class IsolateManager { _vmBreakpointsByIsolateIdAndUri.putIfAbsent(isolateId, () => {}); final existingBreakpointsForIsolateAndUri = existingBreakpointsForIsolate.putIfAbsent(uri, () => []); - await Future.forEach(existingBreakpointsForIsolateAndUri, + // 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. + final breakpointsToRemove = existingBreakpointsForIsolateAndUri.toList(); + existingBreakpointsForIsolateAndUri.clear(); + await Future.forEach(breakpointsToRemove, (bp) => service.removeBreakpoint(isolateId, bp.id!)); // Set new breakpoints. diff --git a/pkg/dds/pubspec.yaml b/pkg/dds/pubspec.yaml index 972dcf19448..bf4a249bb43 100644 --- a/pkg/dds/pubspec.yaml +++ b/pkg/dds/pubspec.yaml @@ -1,5 +1,5 @@ name: dds -version: 2.2.3 +version: 2.2.4 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/debug_breakpoints_test.dart b/pkg/dds/test/dap/integration/debug_breakpoints_test.dart index 3038e061528..0c5ca76f8ce 100644 --- a/pkg/dds/test/dap/integration/debug_breakpoints_test.dart +++ b/pkg/dds/test/dap/integration/debug_breakpoints_test.dart @@ -27,6 +27,50 @@ main() { await client.hitBreakpoint(testFile, breakpointLine); }); + test('does not stop at a removed breakpoint', () async { + final testFile = dap.createTestFile(''' +void main(List args) async { + print('Hello!'); $breakpointMarker + print('Hello!'); $breakpointMarker +} + '''); + + final client = dap.client; + final breakpoint1Line = lineWith(testFile, breakpointMarker); + final breakpoint2Line = breakpoint1Line + 1; + + // Hit the first breakpoint. + final stop = await client.hitBreakpoint(testFile, breakpoint1Line, + additionalBreakpoints: [breakpoint2Line]); + + // Remove all breakpoints. + await client.setBreakpoints(testFile, []); + + // Resume and expect termination (should not hit the second breakpoint). + await Future.wait([ + client.event('terminated'), + client.continue_(stop.threadId!), + ], eagerError: true); + }); + + test('does not fail updating breakpoints after a removal', () async { + // https://github.com/flutter/flutter/issues/106369 was caused by us not + // tracking removals correctly, meaning we could try to remove a removed + // breakpoint a second time. + final client = dap.client; + final testFile = dap.createTestFile(simpleBreakpointProgram); + final breakpointLine = lineWith(testFile, breakpointMarker); + + await client.hitBreakpoint(testFile, breakpointLine); + + // Remove the breakpoint. + await client.setBreakpoints(testFile, []); + + // Send another breakpoint update to ensure it doesn't try to re-remove + // the previously removed breakpoint. + await client.setBreakpoints(testFile, []); + }); + test('stops at a line breakpoint in the SDK set via local sources', () async { final client = dap.client; diff --git a/pkg/dds/test/dap/integration/test_client.dart b/pkg/dds/test/dap/integration/test_client.dart index 4fd429b9504..1dc5b9e0245 100644 --- a/pkg/dds/test/dap/integration/test_client.dart +++ b/pkg/dds/test/dap/integration/test_client.dart @@ -494,18 +494,24 @@ extension DapTestClientExtension on DapTestClient { Future hitBreakpoint( File file, int line, { + List? additionalBreakpoints, File? entryFile, String? condition, String? cwd, List? args, Future Function()? launch, }) async { + assert(condition == null || additionalBreakpoints == null, + 'Only one of condition/additionalBreakpoints can be sent'); entryFile ??= file; final stop = expectStop('breakpoint', file: file, line: line); await Future.wait([ initialize(), - setBreakpoint(file, line, condition: condition), + if (additionalBreakpoints != null) + setBreakpoints(file, [line, ...additionalBreakpoints]) + else + setBreakpoint(file, line, condition: condition), launch?.call() ?? this.launch(entryFile.path, cwd: cwd, args: args), ], eagerError: true); @@ -522,6 +528,16 @@ extension DapTestClientExtension on DapTestClient { ); } + /// Sets breakpoints at [lines] in [file]. + Future setBreakpoints(File file, List lines) async { + await sendRequest( + SetBreakpointsArguments( + source: Source(path: file.path), + breakpoints: lines.map((line) => SourceBreakpoint(line: line)).toList(), + ), + ); + } + /// Sets the exception pause mode to [pauseMode] and expects to pause after /// running the script. ///