Skip to content

Fix a crash during execution context detection #2286

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
1 change: 1 addition & 0 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## 23.0.0-wip
- Restructure `LoadStrategy` to provide build settings. - [#2270](https://github.com/dart-lang/webdev/pull/2270)
- Add `FrontendServerLegacyStrategyProvider` and update bootstrap generation logic for `LegacyStrategy` - [#2285](https://github.com/dart-lang/webdev/pull/2285)
- Tolerate failures to detect a dart execution context. - [#2286](https://github.com/dart-lang/webdev/pull/2286)

## 22.1.0
- Update `package:vm_service` constraint to `^13.0.0`. - [#2265](https://github.com/dart-lang/webdev/pull/2265)
Expand Down
2 changes: 1 addition & 1 deletion dwds/lib/src/connections/debug_connection.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class DebugConnection {
VmService get vmService => _appDebugServices.dwdsVmClient.client;

Future<void> close() => _closed ??= () async {
_appDebugServices.chromeProxyService.remoteDebugger.close();
await _appDebugServices.chromeProxyService.remoteDebugger.close();
await _appDebugServices.close();
_onDoneCompleter.complete();
}();
Expand Down
40 changes: 26 additions & 14 deletions dwds/lib/src/debugging/execution_context.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ import 'package:dwds/src/debugging/remote_debugger.dart';
import 'package:logging/logging.dart';

abstract class ExecutionContext {
/// Returns the context ID that contains the running Dart application.
Future<int> get id;
/// Returns the context ID that contains the running Dart application,
/// if available.
Future<int?> get id;
}

/// The execution context in which to do remote evaluations.
Expand All @@ -23,12 +24,12 @@ class RemoteDebuggerExecutionContext extends ExecutionContext {
/// Context can be null if an error has occurred and we cannot detect
/// and parse the context ID.
late StreamQueue<int> _contexts;
final _contextController = StreamController<int>();
final _seenContexts = <int>[];

int? _id;

@override
Future<int> get id async {
if (_id != null) return _id!;
Future<int?> _lookUpId() async {
_logger.fine('Looking for Dart execution context...');
const timeoutInMs = 100;
while (await _contexts.hasNext.timeout(
Expand All @@ -41,6 +42,7 @@ class RemoteDebuggerExecutionContext extends ExecutionContext {
},
)) {
final context = await _contexts.next;
_seenContexts.add(context);
_logger.fine('Checking context id: $context');
try {
final result = await _remoteDebugger.sendCommand(
Expand All @@ -51,24 +53,34 @@ class RemoteDebuggerExecutionContext extends ExecutionContext {
},
);
if (result.result?['result']?['value'] != null) {
_logger.fine('Found valid execution context: $context');
_id = context;
break;
_logger.fine('Found dart execution context: $context');
return context;
}
} catch (_) {
// Errors may be thrown if we attempt to evaluate in a stale a context.
// Ignore and continue.
_logger.fine('Invalid execution context: $context');
_logger.fine('Stale execution context: $context');
_seenContexts.remove(context);
}
}
return null;
}

@override
Future<int?> get id async {
if (_id != null) return _id;

_id = await _lookUpId();
if (_id == null) {
throw StateError('No context with the running Dart application.');
// Add seen contexts back to the queue in case the injected
// client is still loading, so the next call to `id` succeeds.
_seenContexts.forEach(_contextController.add);
_seenContexts.clear();
}
return _id!;
return _id;
}

RemoteDebuggerExecutionContext(this._id, this._remoteDebugger) {
final contextController = StreamController<int>();
_remoteDebugger
.eventStream('Runtime.executionContextsCleared', (e) => e)
.listen((_) => _id = null);
Expand All @@ -86,8 +98,8 @@ class RemoteDebuggerExecutionContext extends ExecutionContext {
}
return parsedId;
}).listen((e) {
if (e != null) contextController.add(e);
if (e != null) _contextController.add(e);
});
_contexts = StreamQueue(contextController.stream);
_contexts = StreamQueue(_contextController.stream);
}
}
2 changes: 1 addition & 1 deletion dwds/lib/src/debugging/remote_debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -72,5 +72,5 @@ abstract class RemoteDebugger {

Stream<T> eventStream<T>(String method, WipEventTransformer<T> transformer);

void close();
Future<void> close();
}
2 changes: 1 addition & 1 deletion dwds/lib/src/debugging/webkit_debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class WebkitDebugger implements RemoteDebugger {
_wipDebugger.sendCommand(command, params: params);

@override
void close() => _closed ??= _wipDebugger.connection.close();
Future<void> close() => _closed ??= _wipDebugger.connection.close();

@override
Future<void> disable() => _wipDebugger.disable();
Expand Down
15 changes: 14 additions & 1 deletion dwds/lib/src/dwds_vm_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,19 @@ void _recordDwdsStats(DwdsStats dwdsStats, String screen) {
}
}

Future<int> tryGetContextId(
ChromeProxyService chromeProxyService, {
int retries = 3,
}) async {
const waitInMs = 50;
for (var retry = 0; retry < retries; retry++) {
final tryId = await chromeProxyService.executionContext.id;
if (tryId != null) return tryId;
await Future.delayed(const Duration(milliseconds: waitInMs));
}
throw StateError('No context with the running Dart application.');
}

Future<Map<String, dynamic>> _hotRestart(
ChromeProxyService chromeProxyService,
VmService client,
Expand All @@ -223,7 +236,7 @@ Future<Map<String, dynamic>> _hotRestart(
await _disableBreakpointsAndResume(client, chromeProxyService);
try {
_logger.info('Attempting to get execution context ID.');
await chromeProxyService.executionContext.id;
await tryGetContextId(chromeProxyService);
_logger.info('Got execution context ID.');
} on StateError catch (e) {
// We couldn't find the execution context. `hotRestart` may have been
Expand Down
1 change: 1 addition & 0 deletions dwds/lib/src/loaders/legacy.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class LegacyStrategy extends LoadStrategy {
/// packages/path/path -> d348c2a4647e998011fe305f74f22961
///
final Future<Map<String, String>> Function(MetadataProvider metadataProvider)
// ignore: unused_field
_digestsProvider;

/// Returns the module for the corresponding server path.
Expand Down
2 changes: 1 addition & 1 deletion dwds/lib/src/servers/extension_debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ class ExtensionDebugger implements RemoteDebugger {
int newId() => _completerId++;

@override
void close() => _closed ??= () {
Future<void> close() => _closed ??= () {
_closeController.add({});
return Future.wait([
sseConnection.sink.close(),
Expand Down
Loading