Skip to content

Configure DevTools to pause isolates on hot restart, and then only resume once breakpoints have been set #7234

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 26 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1ee9684
Configure pausing isolates on restart
elliette Feb 15, 2024
e579803
Clean up
elliette Feb 16, 2024
37b86cf
onPauseStart only
elliette Feb 16, 2024
4e96028
service is nullable
elliette Mar 5, 2024
347ad86
Merge branch 'master' into pause-isolates
elliette Mar 5, 2024
e554e63
Update release notes
elliette Mar 5, 2024
3077212
Fix analyzer error
elliette Mar 5, 2024
f71d557
Missing semicolon :(
elliette Mar 6, 2024
7b6fb8c
Resume any newly spawned isolates
elliette Mar 7, 2024
fc60b24
Merge branch 'master' into pause-isolates
elliette Mar 7, 2024
79aca85
Detect is pause-isolates-on-start was already set
elliette Mar 7, 2024
a9e0030
Fix resuming for web apps
elliette Mar 26, 2024
ea6e838
Fix resuming for web apps
elliette Mar 26, 2024
17904c7
Use readyToResume
elliette Mar 27, 2024
3d2519a
Use readyToResume
elliette Mar 27, 2024
72a803f
Clean up
elliette Mar 28, 2024
8ba5fb0
Resolve merge conflicts
elliette Apr 15, 2024
9c4d94f
Update dds_service_extensions
elliette Apr 16, 2024
b426437
Hopefully fix integration test
elliette Apr 16, 2024
b42b842
More attempts to fix test
elliette Apr 16, 2024
77dc4d1
More attempts to fix test
elliette Apr 16, 2024
0d81940
Commit in correct directory
elliette Apr 16, 2024
874f223
More attempts to fix test
elliette Apr 16, 2024
6a46af0
Resolve merge conflicts
elliette Apr 16, 2024
49f84c8
Split service_connection_test into service_connection_test and servic…
elliette Apr 17, 2024
6bc4e18
Fix issue where wasn't actually pausing
elliette Apr 17, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,10 @@ class BreakpointManager with DisposerMixin {
isolateRef: isolateRef,
);
}

// Resume the isolate now that the breakpoints have been set:
await serviceConnection.serviceManager.isolateManager
.resumeIsolate(isolateRef);
}

void clearCache({required bool isServiceShutdown}) {
Expand Down
2 changes: 1 addition & 1 deletion packages/devtools_app/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ dependencies:
codicon: ^1.0.0
collection: ^1.15.0
dap: ^1.1.0
dds_service_extensions: ^1.6.0
dds_service_extensions: ^2.0.0
devtools_app_shared: ^0.2.0-dev.0
devtools_extensions: ^0.2.0-dev.0
devtools_shared: ^9.0.1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ TODO: Remove this section if there are not any general updates.

## Debugger updates

TODO: Remove this section if there are not any general updates.
* During a hot-restart, `pause_isolates_on_start` and only `resume` the app once breakpoints are set. - [#7234](https://github.com/flutter/devtools/pull/7234)

## Network profiler updates

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'dart:async';
import 'dart:core';

import 'package:collection/collection.dart' show IterableExtension;
import 'package:dds_service_extensions/dds_service_extensions.dart';
import 'package:flutter/foundation.dart';
import 'package:logging/logging.dart';
import 'package:vm_service/vm_service.dart' hide Error;
Expand Down Expand Up @@ -99,6 +100,13 @@ final class IsolateManager with DisposerMixin {
_isolates.add(isolateRef);
isolateIndex(isolateRef);
await _loadIsolateState(isolateRef);
// If the flag pause-breakpoints-on-start was successfully set, then each
// new isolate will start paused. Therefore resume it (unless it is the
// current isolate, in which case the breakpoint manager will resume it
// after setting breakpoints):
if (selectedIsolate.value?.id != isolateRef.id) {
await resumeIsolate(isolateRef);
}
}

Future<void> _loadIsolateState(IsolateRef isolateRef) async {
Expand Down Expand Up @@ -237,6 +245,32 @@ final class IsolateManager with DisposerMixin {
_isolateRunnableCompleters.clear();
}

/// Resumes the isolate by calling [DdsExtension.readyToResume].
///
/// CAUTION: This should only be used for a tool-initiated resume, not a user-
/// initiated resume. See:
/// https://github.com/dart-lang/sdk/commit/5536951738ba599d96e075b7140e52b28e233
Future<void> resumeIsolate(IsolateRef isolateRef) async {
if (isolateRef.id == null || _service == null) return;
final isolateId = isolateRef.id!;
try {
await _readyToResume(isolateId);
} catch (error) {
_log.warning(error);
}
}

Future<void> _readyToResume(String isolateId) async {
final service = _service!;
try {
await service.readyToResume(isolateId);
} on UnimplementedError {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually throw UnimplementedError on this end? I would assume this would actually result in an RPCError with code kMethodNotFound.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just verified that is does throw UnimplementedError (for both web and non-web apps)

// Fallback to a regular resume if the DDS version doesn't support
// `readyToResume`:
await service.resume(isolateId);
}
}

void _clearIsolateStates() {
for (var isolateState in _isolateStates.values) {
isolateState.dispose();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import 'dart:async';
import 'dart:core';

import 'package:dds_service_extensions/dds_service_extensions.dart';
import 'package:flutter/foundation.dart';
import 'package:logging/logging.dart';
import 'package:meta/meta.dart';
Expand Down Expand Up @@ -238,6 +239,8 @@ class ServiceManager<T extends VmService> {
serviceExtensionManager.vmServiceOpened(service, connectedApp!);
resolvedUriManager.vmServiceOpened(service);

await _configureIsolateSettings();

await callLifecycleCallbacks(
ServiceManagerLifecycle.beforeOpenVmService,
service,
Expand Down Expand Up @@ -278,6 +281,24 @@ class ServiceManager<T extends VmService> {
_connectedState.value = connectionState;
}

Future<void> _configureIsolateSettings() async {
await _setPauseIsolatesOnStart();
}

Future<void> _setPauseIsolatesOnStart() async {
if (service == null) return;
final vmService = service!;

try {
await vmService.setFlag('pause_isolates_on_start', 'true');
await vmService.requirePermissionToResume(
onPauseStart: true,
);
} catch (error) {
_log.warning('$error');
}
}

/// Initializes the service manager for [service], including setting up other
/// managers, initializing stream listeners, and setting up connection state.
Future<void> _openVmServiceConnection(
Expand Down
1 change: 1 addition & 0 deletions packages/devtools_app_shared/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ environment:

dependencies:
collection: ^1.15.0
dds_service_extensions: ^2.0.0
devtools_shared: ^9.0.1
dtd: ^2.1.0
flutter:
Expand Down