Skip to content

Commit 3ab8fca

Browse files
authored
[DWDS] Don't send PauseInterrupted event during a hot reload (#2695)
Fixes dart-lang/sdk#61560 We rely on a pause within a hot reload to pause execution so that we can reregister breakpoints. However, the existing pause mechanism always sends a PauseInterrupted event, which then triggers the client to think this is a normal pause event and not an internal detail. Instead, we should have the ChromeProxyService signal to the debugger that this is an "internal pause" and therefore it should not send a regular pause event and should use a completer to signal the pause is done. Tests are refactored and updated to correctly check for the events when reregistering breakpoints. Specifically, it checks no other events besides the expected ones are sent.
1 parent cdc5cc3 commit 3ab8fca

File tree

6 files changed

+569
-1060
lines changed

6 files changed

+569
-1060
lines changed

dwds/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
- Bump SDK constraint to ^3.10.0
44
- Added 'scriptUri' parameter to compileExpressionToJs
5+
- Fix an issue in `reloadSources` where a `PauseInterrupted` event was sent. - [#61560](https://github.com/dart-lang/sdk/issues/61560)
56

67
## 25.1.0
78

dwds/lib/src/debugging/debugger.dart

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,28 @@ class Debugger extends Domain {
9696
bool _isStepping = false;
9797
DartLocation? _previousSteppingLocation;
9898

99+
// If not null and not completed, the pause handler completes this instead of
100+
// sending a `PauseInterrupted` event to the event stream.
101+
//
102+
// In some cases e.g. a hot reload, DWDS pauses the execution itself in order
103+
// to handle debugging logic like breakpoints. In such cases, sending the
104+
// event to the event stream may trigger the client to believe that the user
105+
// sent the event. To avoid that and still signal that a pause is completed,
106+
// this completer is used. See https://github.com/dart-lang/sdk/issues/61560
107+
// for more details.
108+
Completer<void>? _internalPauseCompleter;
109+
99110
void updateInspector(AppInspectorInterface appInspector) {
100111
inspector = appInspector;
101112
_breakpoints.inspector = appInspector;
102113
}
103114

104-
Future<Success> pause() async {
115+
Future<Success> pause({bool internalPause = false}) async {
105116
_isStepping = false;
117+
if (internalPause) _internalPauseCompleter = Completer<void>();
106118
final result = await _remoteDebugger.pause();
107119
handleErrorIfPresent(result);
120+
await _internalPauseCompleter?.future;
108121
return Success();
109122
}
110123

@@ -632,7 +645,14 @@ class Debugger extends Domain {
632645
// DevTools is showing an overlay. Both cannot be shown at the same time.
633646
// _showPausedOverlay();
634647
isolate.pauseEvent = event;
635-
_streamNotify('Debug', event);
648+
final internalPauseCompleter = _internalPauseCompleter;
649+
if (event.kind == EventKind.kPauseInterrupted &&
650+
internalPauseCompleter != null &&
651+
!internalPauseCompleter.isCompleted) {
652+
internalPauseCompleter.complete();
653+
} else {
654+
_streamNotify('Debug', event);
655+
}
636656
}
637657

638658
/// Handles resume events coming from the Chrome connection.

0 commit comments

Comments
 (0)