Skip to content

[flutter_tools] Make DeviceLogReader.dispose asynchronous - #195389

Merged
auto-submit[bot] merged 3 commits into
flutter:masterfrom
bkonyi:ft-ext-device-log-reader-async-dispose
Nov 3, 2026
Merged

auto-submit[bot] merged 3 commits into
flutter:masterfrom
bkonyi:ft-ext-device-log-reader-async-dispose

Conversation

@bkonyi

@bkonyi bkonyi commented Oct 27, 2026

Copy link
Copy Markdown
Contributor

Updates DeviceLogReader.dispose to return Future<void> and awaits disposal across device log reader implementations and callers.

Follow-up to #194964 (comment)
Part of #190755

Updates DeviceLogReader.dispose to return Future<void> and awaits disposal across device log reader implementations and callers.

Follow-up to flutter#194964 (comment)
Part of flutter#190755
@bkonyi bkonyi added the CICD Run CI/CD label Oct 27, 2026
@github-actions github-actions Bot added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. team-android Owned by Android platform team team-ios Owned by iOS platform team labels Oct 27, 2026
@bkonyi
bkonyi requested a review from loic-sharma October 27, 2026 19:17
@bkonyi
bkonyi marked this pull request as ready for review October 27, 2026 19:17
@bkonyi
bkonyi requested review from a team as code owners October 27, 2026 19:17
@bkonyi
bkonyi requested review from reidbaker and removed request for a team October 27, 2026 19:17

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors DeviceLogReader.dispose and related cleanup methods across various device and test classes to return Future<void> instead of void, ensuring resources are properly awaited during disposal. The review feedback identifies invalid Dart syntax (using a prefix ? in collection literals) in IOSDeviceLogReader and CoreDeviceLoggingSource, and recommends using Future.wait instead of sequential awaits in several dispose implementations to guarantee all cleanup operations are initiated even if one fails or blocks.

Use `Future.wait` to dispose `DeviceLogReader` and `DevicePortForwarder` instances concurrently in `AndroidDevice.dispose`, `FlutterTesterDevice.dispose`, `ChromiumDevice.dispose`, and `WebServerDevice.dispose`.

@loic-sharma loic-sharma left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 28, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 28, 2026
@auto-submit

auto-submit Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/195389, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@bkonyi
bkonyi requested a review from loic-sharma October 28, 2026 02:54
@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 28, 2026
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Oct 28, 2026
@auto-submit

auto-submit Bot commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/195389, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 3, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Nov 3, 2026
Merged via the queue into flutter:master with commit 287ec89 Nov 3, 2026
29 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 3, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Nov 4, 2026
flutter/flutter@9347202...3623d2d

2026-11-04 [email protected] Roll Skia from 49ec2655bf51 to aaa39bfcd393 (5 revisions) (flutter/flutter#195956)
2026-11-04 [email protected] Roll pub packages (flutter/flutter#195950)
2026-11-04 [email protected] Roll pub packages (flutter/flutter#195948)
2026-11-04 [email protected] local_engine_builds_common out of bringup (flutter/flutter#195862)
2026-11-04 [email protected] [web][a11y] Initialize slider semantics values when enabling (flutter/flutter#195564)
2026-11-04 [email protected] Drop Android SDK, JDK, and Gradle dependencies from framework_tests_misc (flutter/flutter#195869)
2026-11-04 [email protected] Roll Fuchsia Test Scripts from B6zarykZp6PKWRueg... to Jo-ZgQSNDuPsjlZ4s... (flutter/flutter#195937)
2026-11-04 [email protected] Balance tool_tests_commands and web_tool_tests subshards by duration (flutter/flutter#195863)
2026-11-04 [email protected] Roll Fuchsia Linux SDK from T_-6-6X1lDMghhAvX... to p0HDXLISPIElJfSz7... (flutter/flutter#195939)
2026-11-03 [email protected] Roll Skia from 3eeafac94f07 to 49ec2655bf51 (8 revisions) (flutter/flutter#195933)
2026-11-03 [email protected] Roll pub packages (flutter/flutter#195932)
2026-11-03 [email protected] [flutter_tools] Lazily initialize AppleContext dependencies via builder closures (flutter/flutter#191992)
2026-11-03 [email protected] [Android] Upgrade linux_android_emu_vulkan_stable avd_cipd_version to 36.1.9 (build_id:8670032186093418337) (flutter/flutter#195856)
2026-11-03 [email protected] [vulkan]: adds "color attachment read" dst dependency to subpass (flutter/flutter#195714)
2026-11-03 [email protected] [flutter_tools] Stream web dev server proxy responses instead of buffering them (flutter/flutter#194934)
2026-11-03 [email protected] Roll Skia from d80db8f8f856 to 3eeafac94f07 (6 revisions) (flutter/flutter#195926)
2026-11-03 [email protected] [flutter_tools] Add flutter assemble support for tool extension assemble targets (flutter/flutter#194563)
2026-11-03 [email protected] Manual roll Dart SDK from fe609902984b to e362a23e9738 (2 revisions) (flutter/flutter#195914)
2026-11-03 [email protected] feat(flutter_test): add startHover and startHoverAt helper to WidgetTester (flutter/flutter#190630)
2026-11-03 [email protected] [flutter_tools] Remove trailing dots from bug report URLs (flutter/flutter#194132)
2026-11-03 [email protected] Update skills_lint to 0.5.3 (flutter/flutter#195796)
2026-11-03 [email protected] Roll Skia from 58295d932350 to d80db8f8f856 (5 revisions) (flutter/flutter#195900)
2026-11-03 [email protected] [flutter_tools] Make DeviceLogReader.dispose asynchronous (flutter/flutter#195389)
2026-11-03 [email protected] Roll Packages from 3f34a6f to 1df8447 (10 revisions) (flutter/flutter#195895)
2026-11-03 [email protected] Roll Skia from c733ad3e37cb to 58295d932350 (1 revision) (flutter/flutter#195894)

If this roll has caused a breakage, revert this CL and set the roller
to dry run mode using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD platform-ios iOS applications specifically team-android Owned by Android platform team team-ios Owned by iOS platform team tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants