Repository navigation
[flutter_tools] Use ErrorHandlingFileSystem.deleteIfExists for IOSCoreDeviceControl temp directory cleanup - #195913
Conversation
…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! ```
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
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
In
IOSCoreDeviceControl,getCoreDevices(_listCoreDevices) usedErrorHandlingFileSystem.deleteIfExists(tempDirectory, recursive: true)in itsfinallyblock (added in #140415 for #140416), while the remaining 7devicectlhelper methods (_listInstalledApps,installApp,uninstallApp,launchApp,terminateProcess,_listRunningProcesses, andtakeScreenshot) calledtempDirectory.deleteSync(recursive: true)directly (withtakeScreenshotwrapping it intry ... on FileSystemException), as tracked in #186954 and #186959.If
LocalFileSystemsignal cleanup (_tryToDeleteTemponSIGINT/SIGTERM, as seen in the siblingbuildXcodeProjecttemp-directory deletion issue #195906) or external temp cleanup deletes the per-process temp root whilexcrun devicectlis running,ErrorHandlingDirectory.deleteSyncrethrowsENOENT(errno = 2) as an unhandledPathNotFoundExceptionfrom thefinallyblock becauseignoreErrorCodesbypassesToolExitconversion forkSystemCodeCannotFindFile.This change replaces the remaining 7
tempDirectory.deleteSync(recursive: true)calls infinallyblocks inpackages/flutter_tools/lib/src/ios/core_devices.dartwithErrorHandlingFileSystem.deleteIfExists(tempDirectory, recursive: true)(which also narrowstakeScreenshot's cleanup to ignoreENOENTrather than allFileSystemExceptions) and adds unit tests simulating temp directory deletion duringdevicectlexecution.Part of #186959
Previous output (unit test simulating temp directory deletion during
devicectlbefore fix):Current output:
Pre-launch Checklist
///).