Skip to content

Fix race condition on simultaneous hot restarts #1870

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 14 commits into from
Jan 10, 2023
14 changes: 8 additions & 6 deletions dwds/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,26 +1,28 @@
## 16.0.2-dev
## 17.0.0-dev

- Include debug information in the event sent from the injected client to the
Dart Debug Extension notifying that the Dart app is ready.
- Include an optional param to `Dwds.start` to indicate whether it is running
internally or externally.
- Fix null cast error on expression evaluations after dwds fails to find class
metadata.
- Include the entire exception description up to the stacktrace in
`mapExceptionStackTrace`.
- Allow enabling experiments in the expression compiler service.
- Include an optional param to `Dwds.start` to indicate whether it a Flutter app
or not.
- Pre-warm expression compiler cache to speed up Flutter Inspector loading.
- Remove `ChromeProxyService.setExceptionPauseMode()`.
- Display full error on failure to start DDS.
- Fix crash on processing DevTools event when starting DevTools from DevTools
uri.
- Prepare or Dart 3 alpha breaking changes:
- Move weak null safety tests to special branch of `build_web_compilers`.
- Do not pass `--(no)-sound-null-safety` flag to build daemon.
- Add back `ChromeProxyService.setExceptionPauseMode()` without override.
- Make hot restart atomic to prevent races on simultaneous execution.

**Breaking changes**
- Include an optional param to `Dwds.start` to indicate whether it is running
internally or externally.
- Include an optional param to `Dwds.start` to indicate whether it a Flutter app
or not.
- Remove deprecated `ChromeProxyService.setExceptionPauseMode()`.

## 16.0.1

Expand Down
6 changes: 3 additions & 3 deletions dwds/lib/src/debugging/debugger.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
import 'dart:async';
import 'dart:math' as math;

import 'package:dwds/src/utilities/synchronized.dart';
import 'package:logging/logging.dart';
import 'package:pool/pool.dart';
import 'package:vm_service/vm_service.dart';
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
hide StackTrace;
Expand Down Expand Up @@ -779,7 +779,7 @@ class _Breakpoints extends Domain {

final _bpByDartId = <String, Future<Breakpoint>>{};

final _pool = Pool(1);
final _queue = AtomicQueue();

final Locations locations;
final RemoteDebugger remoteDebugger;
Expand Down Expand Up @@ -868,7 +868,7 @@ class _Breakpoints extends Domain {
final urlRegex = '.*${location.jsLocation.module}.*';
// Prevent `Aww, snap!` errors when setting multiple breakpoints
// simultaneously by serializing the requests.
return _pool.withResource(() async {
return _queue.run(() async {
final breakPointId = await sendCommandAndValidateResult<String>(
remoteDebugger,
method: 'Debugger.setBreakpointByUrl',
Expand Down
10 changes: 5 additions & 5 deletions dwds/lib/src/debugging/frame_computer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'package:pool/pool.dart';
import 'package:dwds/src/utilities/synchronized.dart';
import 'package:vm_service/vm_service.dart';
import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart';

Expand All @@ -11,9 +11,9 @@ import 'debugger.dart';
class FrameComputer {
final Debugger debugger;

// To ensure that the frames are computed only once, we use a pool to guard
// the work. Frames are computed sequentially.
final _pool = Pool(1);
// To ensure that the frames are computed only once, we use an atomic queue
// to guard the work. Frames are computed sequentially.
final _queue = AtomicQueue();

final List<WipCallFrame> _callFrames;
final List<Frame> _computedFrames = [];
Expand All @@ -36,7 +36,7 @@ class FrameComputer {
/// Translates Chrome callFrames contained in [DebuggerPausedEvent] into Dart
/// [Frame]s.
Future<List<Frame>> calculateFrames({int? limit}) async {
return _pool.withResource(() async {
return _queue.run(() async {
if (limit != null && _computedFrames.length >= limit) {
return _computedFrames.take(limit).toList();
}
Expand Down
16 changes: 14 additions & 2 deletions dwds/lib/src/dwds_vm_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'dart:async';
import 'dart:convert';

import 'package:dwds/src/utilities/synchronized.dart';
import 'package:logging/logging.dart';
import 'package:uuid/uuid.dart';
import 'package:vm_service/vm_service.dart';
Expand Down Expand Up @@ -32,6 +33,9 @@ class DwdsVmClient {
/// All subsequent calls to [close] will return this future.
Future<void>? _closed;

/// Synchronizes hot restarts to avoid races.
final _hotRestartQueue = AtomicQueue();

DwdsVmClient(this.client, this._requestController, this._responseController);

Future<void> close() => _closed ??= () async {
Expand Down Expand Up @@ -60,6 +64,9 @@ class DwdsVmClient {
final chromeProxyService =
debugService.chromeProxyService as ChromeProxyService;

final dwdsVmClient =
DwdsVmClient(client, requestController, responseController);

// Register '_flutter.listViews' method on the chrome proxy service vm.
// In native world, this method is provided by the engine, but the web
// engine is not aware of the VM uri or the isolates.
Expand All @@ -85,7 +92,7 @@ class DwdsVmClient {
client.registerServiceCallback(
'hotRestart',
(request) => captureElapsedTime(
() => _hotRestart(chromeProxyService, client),
() => dwdsVmClient.hotRestart(chromeProxyService, client),
(_) => DwdsEvent.hotRestart()));
await client.registerService('hotRestart', 'DWDS');

Expand Down Expand Up @@ -138,7 +145,12 @@ class DwdsVmClient {
});
await client.registerService('_yieldControlToDDS', 'DWDS');

return DwdsVmClient(client, requestController, responseController);
return dwdsVmClient;
}

Future<Map<String, dynamic>> hotRestart(
ChromeProxyService chromeProxyService, VmService client) async {
return _hotRestartQueue.run(() => _hotRestart(chromeProxyService, client));
}
}

Expand Down
Loading