Skip to content

[flutter_tools] Use ErrorHandlingFileSystem.deleteIfExists for IOSCoreDeviceControl temp directory cleanup - #195913

Merged
auto-submit[bot] merged 2 commits into
flutter:masterfrom
bkonyi:fix-core-devices-temp-dir-delete
Nov 6, 2026
Merged

auto-submit[bot] merged 2 commits into
flutter:masterfrom
bkonyi:fix-core-devices-temp-dir-delete

Conversation

@bkonyi

@bkonyi bkonyi commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

In IOSCoreDeviceControl, getCoreDevices (_listCoreDevices) used ErrorHandlingFileSystem.deleteIfExists(tempDirectory, recursive: true) in its finally block (added in #140415 for #140416), while the remaining 7 devicectl helper methods (_listInstalledApps, installApp, uninstallApp, launchApp, terminateProcess, _listRunningProcesses, and takeScreenshot) called tempDirectory.deleteSync(recursive: true) directly (with takeScreenshot wrapping it in try ... on FileSystemException), as tracked in #186954 and #186959.

If LocalFileSystem signal cleanup (_tryToDeleteTemp on SIGINT/SIGTERM, as seen in the sibling buildXcodeProject temp-directory deletion issue #195906) or external temp cleanup deletes the per-process temp root while xcrun devicectl is running, ErrorHandlingDirectory.deleteSync rethrows ENOENT (errno = 2) as an unhandled PathNotFoundException from the finally block because ignoreErrorCodes bypasses ToolExit conversion for kSystemCodeCannotFindFile.

This change replaces the remaining 7 tempDirectory.deleteSync(recursive: true) calls in finally blocks in packages/flutter_tools/lib/src/ios/core_devices.dart with ErrorHandlingFileSystem.deleteIfExists(tempDirectory, recursive: true) (which also narrows takeScreenshot's cleanup to ignore ENOENT rather than all FileSystemExceptions) and adds unit tests simulating temp directory deletion during devicectl execution.

Part of #186959

Previous output (unit test simulating temp directory deletion during devicectl before fix):

+34 -1: Core Device Control install app Handles FileSystemException deleting temp directory [E]

  FileSystemException: No such file or directory, path = '/.tmp_rand0/core_devices.rand0' (OS Error: No such file or directory, errno = 2)

Current output:

+87: All tests passed!

Pre-launch Checklist

…CoreDeviceControl` temp directory cleanup

In `IOSCoreDeviceControl`, `getCoreDevices` (`_listCoreDevices`) used `ErrorHandlingFileSystem.deleteIfExists(tempDirectory, recursive: true)` in its `finally` block (added in flutter#140415), while the remaining 7 `devicectl` helper methods (`_listInstalledApps`, `installApp`, `uninstallApp`, `launchApp`, `terminateProcess`, `_listRunningProcesses`, and `takeScreenshot`) called `tempDirectory.deleteSync(recursive: true)` directly (with `takeScreenshot` wrapping it in `try ... on FileSystemException`).

If `LocalFileSystem` signal cleanup (`_tryToDeleteTemp` on `SIGINT`/`SIGTERM`, as seen in flutter#195733) or external temp cleanup deletes the per-process temp root while `xcrun devicectl` is running, `ErrorHandlingDirectory.deleteSync` rethrows `ENOENT` (`errno = 2`) as an unhandled `PathNotFoundException` from the `finally` block because `ignoreErrorCodes` bypasses `ToolExit` conversion for `kSystemCodeCannotFindFile`.

Replace the remaining 7 `tempDirectory.deleteSync(recursive: true)` calls in `finally` blocks in `packages/flutter_tools/lib/src/ios/core_devices.dart` with `ErrorHandlingFileSystem.deleteIfExists(tempDirectory, recursive: true)` (which also narrows `takeScreenshot`'s cleanup to ignore `ENOENT` rather than all `FileSystemException`s) and add unit tests simulating temp directory deletion during `devicectl` execution.

Previous output (unit test simulating temp directory deletion during `devicectl` before fix):
```
+34 -1: Core Device Control install app Handles FileSystemException deleting temp directory [E]

  FileSystemException: No such file or directory, path = '/.tmp_rand0/core_devices.rand0' (OS Error: No such file or directory, errno = 2)
```

Current output:
```
+87: All tests passed!
```
@bkonyi bkonyi added the CICD Run CI/CD label Nov 3, 2026
@github-actions github-actions Bot added platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. team-ios Owned by iOS platform team labels Nov 3, 2026
@bkonyi
bkonyi marked this pull request as ready for review November 3, 2026 20:11
@bkonyi
bkonyi requested a review from a team as a code owner November 3, 2026 20:11

@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 updates IOSCoreDeviceControl in core_devices.dart to use ErrorHandlingFileSystem.deleteIfExists instead of deleteSync when cleaning up temporary directories in finally blocks, ensuring that FileSystemException errors are safely handled. Corresponding unit tests have been added in core_devices_test.dart to verify this behavior across multiple device control operations. There are no review comments, and I have no feedback to provide.

…deletion test

Match the realistic devicectl interruption path (exitCode: 1) in the getRunningProcesses temp directory deletion unit test so it exercises finally block cleanup after ProcessException rather than raw MemoryFileSystem readAsStringSync.
} on FileSystemException {
// Ignore.
}
ErrorHandlingFileSystem.deleteIfExists(tempDirectory, recursive: true);

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.

Does this suppress exceptions?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It suppresses ENOENT / path-not-found (both upfront and if the directory disappears mid-delete), while other FileSystemExceptions still rethrow or become a ToolExit via ErrorHandlingDirectory, which matches _listCoreDevices and the other helpers here.

@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2026
@auto-submit
auto-submit Bot added this pull request to the merge queue Nov 6, 2026
Merged via the queue into flutter:master with commit 2a61cbe Nov 6, 2026
23 checks passed
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 6, 2026
auto-submit Bot pushed a commit to flutter/packages that referenced this pull request Nov 7, 2026
flutter/flutter@09175cd...e36b370

2026-11-07 [email protected] Roll Fuchsia Linux SDK from KIc0XZbhLmvDpTluZ... to Sf2yvdTiFXAKkfLkD... (flutter/flutter#196160)
2026-11-07 [email protected] Roll pub packages (flutter/flutter#196155)
2026-11-07 [email protected] [iOS a11y] Use a real `UIScrollView` as a11y scrollable container (flutter/flutter#195731)
2026-11-07 [email protected] Add ParagraphStyle.hyphenCharacter to dart:ui (flutter/flutter#195345)
2026-11-07 [email protected] Roll pub packages (flutter/flutter#196143)
2026-11-06 [email protected] [Linux] Fix layer position when compositing without glBlitFramebuffer (flutter/flutter#196083)
2026-11-06 [email protected] Add support for riscv64 desktop linux engine (flutter/flutter#178712)
2026-11-06 [email protected] Add ResizeImagePolicy.cover (flutter/flutter#194665)
2026-11-06 [email protected] Roll Skia from f533d4034900 to b11315d0a096 (6 revisions) (flutter/flutter#196138)
2026-11-06 [email protected] iOS: Remove dead code/unused imports from FlutterPlatformViewsController (flutter/flutter#196084)
2026-11-06 [email protected] Replace pull_request_target workflows with pull_request + workflow_run (flutter/flutter#195536)
2026-11-06 [email protected] [master] Update Flutter DEPS to Dart 3b174b7217fe09b21da836ea623932b5f041b091 (flutter/flutter#196004)
2026-11-06 [email protected] Fix android_hardware_smoke_test retry mechanisms (flutter/flutter#195708)
2026-11-06 [email protected] Roll Skia from 452b9c1e5db6 to f533d4034900 (1 revision) (flutter/flutter#196129)
2026-11-06 [email protected] [flutter_tools] Use `ErrorHandlingFileSystem.deleteIfExists` for `IOSCoreDeviceControl` temp directory cleanup (flutter/flutter#195913)
2026-11-06 [email protected] Adds guideline tests to flutter create samples (flutter/flutter#195827)
2026-11-06 [email protected] refactor(flutter_tools): unify OptionDescriptor identity and addTo (flutter/flutter#196076)
2026-11-06 [email protected] Roll pub packages (flutter/flutter#196114)
2026-11-06 [email protected] Roll Skia from 5ce64e63a0e7 to 452b9c1e5db6 (5 revisions) (flutter/flutter#196111)
2026-11-06 [email protected] Add PlatformDispatcher.onAlwaysUse24HourFormatChanged callback (flutter/flutter#194670)
2026-11-06 [email protected] Roll Packages from 0478162 to 63881f9 (10 revisions) (flutter/flutter#196108)
2026-11-06 [email protected] Roll Skia from 793b8ac73d4b to 5ce64e63a0e7 (4 revisions) (flutter/flutter#196107)

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-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