Skip to content

Fix getStack race conditions on isolate start #1704

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@
evaluation using a compiler in all scenarios.
- Fix a bug where evaluation would fail with more than one parameter in
the scope.
- Remove showing uncaptured values from the stack during evaluation.
- Remove showing un-captured values from the stack during evaluation.
- Refactor code to break most circular dependencies between files.
- Migrate `package:dwds` to null safety.
- Make `ChromeProxyService.getStack` wait for the debugger to perform initial
resume operation. This avoids race conditions on isolate start.

**Breaking changes**
- Remove no longer used `ExpressionCompilerService.handler`.
Expand Down
2 changes: 1 addition & 1 deletion dwds/lib/src/debugging/debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ class Debugger extends Domain {

/// Returns the current Dart stack for the paused debugger.
///
/// Returns null if the debugger is not paused.
/// Throws RPCError if the debugger is not paused.
///
/// The returned stack will contain up to [limit] frames if provided.
Future<Stack> getStack({int? limit}) async {
Expand Down
10 changes: 9 additions & 1 deletion dwds/lib/src/services/chrome_proxy_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,10 @@ class ChromeProxyService implements VmServiceInterface {
Future<void> get isInitialized => _initializedCompleter.future;
Completer<void> _initializedCompleter = Completer<void>();

/// Signals when isolate starts.
Future<void> get isStarted => _startedCompleter.future;
Completer<void> _startedCompleter = Completer<void>();

/// Signals when expression compiler is ready to evaluate.
Future<void> get isCompilerInitialized => _compilerCompleter.future;
Completer<void> _compilerCompleter = Completer<void>();
Expand Down Expand Up @@ -252,6 +256,7 @@ class ChromeProxyService implements VmServiceInterface {

unawaited(appConnection.onStart.then((_) async {
await debugger.resumeFromStart();
_startedCompleter.complete();
}));

final isolateRef = inspector.isolateRef;
Expand Down Expand Up @@ -301,6 +306,7 @@ class ChromeProxyService implements VmServiceInterface {
final isolateRef = inspector.isolateRef;

_initializedCompleter = Completer<void>();
_startedCompleter = Completer<void>();
_compilerCompleter = Completer<void>();
_streamNotify(
'Isolate',
Expand Down Expand Up @@ -573,12 +579,13 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension(

/// Returns the current stack.
///
/// Returns null if the corresponding isolate is not paused.
/// Throws RPCError the corresponding isolate is not paused.
///
/// The returned stack will contain up to [limit] frames if provided.
@override
Future<Stack> getStack(String isolateId, {int? limit}) async {
await isInitialized;
await isStarted;
_checkIsolate('getStack', isolateId);
return (await debuggerFuture).getStack(limit: limit);
}
Expand Down Expand Up @@ -718,6 +725,7 @@ ${globalLoadStrategy.loadModuleSnippet}("dart_sdk").developer.invokeExtension(
if (inspector.appConnection.isStarted) {
return captureElapsedTime(() async {
await isInitialized;
await isStarted;
_checkIsolate('resume', isolateId);
return await (await debuggerFuture)
.resume(step: step, frameIndex: frameIndex);
Expand Down