Skip to content

Killed isolate still holding files it created on windows #120535

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

Open
mgenware opened this issue Feb 11, 2023 · 7 comments
Open

Killed isolate still holding files it created on windows #120535

mgenware opened this issue Feb 11, 2023 · 7 comments
Labels
blocked Issue is blocked by another issue c: crash Stack traces logged to the console dependency: dart Dart team may need to help us dependency:dart-triaged Triaged by Dart team engine flutter/engine repository. See also e: labels. found in release: 3.7 Found to occur in 3.7 found in release: 3.8 Found to occur in 3.8 has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list platform-windows Building on or for Windows specifically team-engine Owned by Engine team triaged-engine Triaged by Engine team

Comments

@mgenware
Copy link

Go to this repo for a reproducible demo. Build and run the app:

1

Tap "start isolate". It generates a tmp file path on main thread and passes it to a new isolate, which then creates the file and writes data to it periodically.

Wait a few secs and tap "stop isolate". The running isolate should be killed and main thread gets notifed and then tries to delete the tmp file. On macOS / iOS / Android, the tmp file can be successfully deleted like this:

2

But on windows, it 100% failed with:

the process cannot access the file because it is being used by another process. errno = 32)

image

The killed isolate seems to still hold the tmp file and prevent other processes from accessing it.

flutter doctor
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.7.3, on Microsoft Windows [Version 10.0.22621.1105], locale en-US)
[✓] Windows Version (Installed version of Windows is version 10 or higher)
[✓] Android toolchain - develop for Android devices (Android SDK version 33.0.1)
[✓] Chrome - develop for the web
[✓] Visual Studio - develop for Windows (Visual Studio Enterprise 2022 17.4.4)
[✓] Android Studio (version 2022.1)
[✓] VS Code (version 1.75.1)
[✓] Connected device (3 available)
[✓] HTTP Host Availability

• No issues found!
@huycozy huycozy added the in triage Presently being triaged by the triage team label Feb 13, 2023
@huycozy
Copy link
Member

huycozy commented Feb 13, 2023

@mgenware Thanks for filing the issue. This issue is reproducible on the latest stable and master channels.

Exception log
flutter: FileSystemException: Cannot delete file, path = 'C:\Users\ADMIN\AppData\Local\Temp\tmp_479dafefc0fe40c1991e6279afe8c653_1676282914646' (OS Error: The process cannot access the file because it is being used by another process.
, errno = 32)
flutter doctor -v (stable & master)
[√] Flutter (Channel stable, 3.7.3, on Microsoft Windows [Version 10.0.19045.2486], locale en-US)
    • Flutter version 3.7.3 on channel stable at C:\WIP\flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 9944297138 (4 days ago), 2023-02-08 15:46:04 -0800
    • Engine revision 248290d6d5
    • Dart version 2.19.2
    • DevTools version 2.20.1

[√] Windows Version (Installed version of Windows is version 10 or higher)

[√] Android toolchain - develop for Android devices (Android SDK version 32.0.0)
    • Android SDK at C:\AndroidSDK
    • Platform android-32, build-tools 32.0.0
    • ANDROID_HOME = C:\AndroidSDK
    • Java binary at: C:\Program Files\Android\Android Studio\jre\bin\java
    • Java version OpenJDK Runtime Environment (build 11.0.12+7-b1504.28-7817840)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Community 2022 17.4.4)
    • Visual Studio at D:\DOWNLOADWORK\VS2022
    • Visual Studio Community 2022 version 17.4.33213.308
    • Windows 10 SDK version 10.0.19041.0

[√] Android Studio (version 2021.2)
    • Android Studio at C:\Program Files\Android\Android Studio
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.12+7-b1504.28-7817840)

[!] Android Studio (version 2022.1)
    • Android Studio at C:\Program Files\Android\Android Studio Electric Eel 2022.1.1
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    X Unable to find bundled Java version.
    • Try updating or re-installing Android Studio.

[√] VS Code (version 1.75.0)
    • VS Code at C:\Users\ADMIN\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.58.0

[√] Connected device (3 available)
    • Windows (desktop) • windows • windows-x64    • Microsoft Windows [Version 10.0.19045.2486]
    • Chrome (web)      • chrome  • web-javascript • Google Chrome 109.0.5414.120
    • Edge (web)        • edge    • web-javascript • Microsoft Edge 110.0.1587.41

[√] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.
[!] Flutter (Channel master, 3.8.0-10.0.pre.34, on Microsoft Windows [Version 10.0.19045.2486], locale en-US)
    • Flutter version 3.8.0-10.0.pre.34 on channel master at C:\WIP\flutter_master
    ! Warning: `flutter` on your path resolves to C:\WIP\flutter\bin\flutter, which is not inside your current
      Flutter SDK checkout at C:\WIP\flutter_master. Consider adding C:\WIP\flutter_master\bin to the front of your
      path.
    ! Warning: `dart` on your path resolves to C:\WIP\flutter\bin\dart, which is not inside your current Flutter SDK
      checkout at C:\WIP\flutter_master. Consider adding C:\WIP\flutter_master\bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 53fe8a3f94 (2 hours ago), 2023-02-13 02:37:24 -0500
    • Engine revision 4107a7b71f
    • Dart version 3.0.0 (build 3.0.0-231.0.dev)
    • DevTools version 2.21.1
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git"
      directly to perform update checks and upgrades.

[√] Windows Version (Installed version of Windows is version 10 or higher)

Checking Android licenses is taking an unexpectedly long time...[√] Android toolchain - develop for Android devices (Android SDK version 32.0.0)
    • Android SDK at C:\AndroidSDK
    • Platform android-32, build-tools 32.0.0
    • ANDROID_HOME = C:\AndroidSDK
    • Java binary at: C:\Program Files\Android\Android Studio Electric Eel 2022.1.1\jbr\bin\java
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)
    • All Android licenses accepted.

[√] Chrome - develop for the web
    • Chrome at C:\Program Files (x86)\Google\Chrome\Application\chrome.exe

[√] Visual Studio - develop for Windows (Visual Studio Community 2022 17.4.4)
    • Visual Studio at D:\DOWNLOADWORK\VS2022
    • Visual Studio Community 2022 version 17.4.33213.308
    • Windows 10 SDK version 10.0.19041.0

[√] Android Studio (version 2021.2)
    • Android Studio at C:\Program Files\Android\Android Studio
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.12+7-b1504.28-7817840)

[√] Android Studio (version 2022.1)
    • Android Studio at C:\Program Files\Android\Android Studio Electric Eel 2022.1.1
    • Flutter plugin can be installed from:
       https://plugins.jetbrains.com/plugin/9212-flutter
    • Dart plugin can be installed from:
       https://plugins.jetbrains.com/plugin/6351-dart
    • Java version OpenJDK Runtime Environment (build 11.0.13+0-b1751.21-8125866)

[√] VS Code (version 1.75.0)
    • VS Code at C:\Users\ADMIN\AppData\Local\Programs\Microsoft VS Code
    • Flutter extension version 3.58.0

[√] Connected device (3 available)
    • Windows (desktop) • windows • windows-x64    • Microsoft Windows [Version 10.0.19045.2486]
    • Chrome (web)      • chrome  • web-javascript • Google Chrome 109.0.5414.120
    • Edge (web)        • edge    • web-javascript • Microsoft Edge 110.0.1587.41

[√] HTTP Host Availability
    • All required HTTP hosts are available

! Doctor found issues in 1 category.

@huycozy huycozy added engine flutter/engine repository. See also e: labels. dependency: dart Dart team may need to help us platform-windows Building on or for Windows specifically has reproducible steps The issue has been confirmed reproducible and is ready to work on found in release: 3.7 Found to occur in 3.7 found in release: 3.8 Found to occur in 3.8 c: crash Stack traces logged to the console and removed in triage Presently being triaged by the triage team labels Feb 13, 2023
@mraleph
Copy link
Member

mraleph commented Feb 13, 2023

/cc @mkustermann @aam

@mkustermann
Copy link
Member

The running isolate should be killed and main thread gets notifed and then tries to delete the tmp file. On macOS / iOS / Android, the tmp file can be successfully deleted like this:

The issue just manifests differently on windows but is there on macOS / iOS / Android. Windows is more strict in deletion APIs - it only allows deleting a directory when it's not used anymore. Linux allows deleting it while it's still being used (and will free it internally in kernel only after last use is done).

(The Dart VM itself doesn't know anything about dart:io - it's something the embedder e.g. flutter / standalone native runtime know about.)

If the file isn't explicitly deleted by Dart code, it's the embedders responsibility to decide what to do. The dart:io support in our embedder will attach finalizers to the files after they're created and the files will therefore be closed in the finalizer only after GC has collected the file objects.

To demonstrate this, here's an example that runs isolates and afterwards prints the list of open files

import 'dart:isolate';
import 'dart:io';

main() async {
  for (int i = 0; i < 100; ++i) {
    print('$i');
    await Isolate.run(() {
      File('/tmp/foo.$i').openSync(mode: FileMode.write);
      return null;
    });
  }
  print('done');
  for (final line in Process.runSync('/usr/bin/ls', ['-l', '/proc/${pid}/fd'])
      .stdout
      .split('\n')) {
    if (line.contains('/tmp/foo')) {
      print(line);
    }
  }
}

When running with --verbose-gc we can see when GC happens:

% dart --verbose-gc test.dart
0
1
2
...
45
46
47
[ main         ,    Scavenge(   new space),    3,   0.29,   1.2,   1.1,   0.1,   2.0,   2.0, 0.0, 0.0,    5.0,    5.0,    5.8,    5.8,   0.0,   0.0,   0,   1,  -1.0,    0.0, ]
48
49
50
...
97
98
99
done
lrwx------ 1 ...  -> /tmp/foo.47
lrwx------ 1 ...  -> /tmp/foo.48
lrwx------ 1 ...  -> /tmp/foo.49
...
lrwx------ 1 ...  -> /tmp/foo.98
lrwx------ 1 ...  -> /tmp/foo.99

So this is somewhat working as intended. Just like if a single isolate doesn't close an unused file, it will eventually be closed via GC.

@mraleph
Copy link
Member

mraleph commented Feb 13, 2023

@mkustermann I think this is somewhat of a regression from the previous behaviour (pre-isolate groups). Given that we don't support passing native resources like files and sockets between isolates we might want to provide embedder APIs that allow prompt finalization of those resources and migrate dart:io to such APIs.

@mkustermann
Copy link
Member

mkustermann commented Feb 13, 2023

@mkustermann I think this is somewhat of a regression from the previous behaviour (pre-isolate groups). Given that we don't support passing native resources like files and sockets between isolates we might want to provide embedder APIs that allow prompt finalization of those resources and migrate dart:io to such APIs.

One could say that, yes - because before it happens to be that destruction of isolate meant destruction of GC'ed heap, which would run finalizers.

We can decide to avoid relying purely on GC and instead eagerly finalize native resources on isolate death.

Though I'd like to point out that it won't entirely solve the issue, since one can actually have a leak that even GC won't even fix, due to file creation + attaching of finalizers being done non-atomically: It can be interrupted via Isolate.kill() and will cause process-wide leak.

=> Filed dart-lang/sdk#51387 to track discussion & implementation.

@chinmaygarde chinmaygarde added the P2 Important issues not at the top of the work list label Feb 13, 2023
@flutter-triage-bot flutter-triage-bot bot added team-engine Owned by Engine team triaged-engine Triaged by Engine team labels Jul 8, 2023
@a-siva a-siva added the blocked Issue is blocked by another issue label Feb 1, 2024
@a-siva
Copy link
Contributor

a-siva commented Feb 1, 2024

This is blocked on dart-lang/sdk#51387

@a-siva a-siva added the dependency:dart-triaged Triaged by Dart team label Feb 8, 2024
@brianquinlan
Copy link
Contributor

@chinmaygarde

Is this really a P2? Do we set the expectation somewhere that system resources will be eagerly reclaimed when an isolate is killed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Issue is blocked by another issue c: crash Stack traces logged to the console dependency: dart Dart team may need to help us dependency:dart-triaged Triaged by Dart team engine flutter/engine repository. See also e: labels. found in release: 3.7 Found to occur in 3.7 found in release: 3.8 Found to occur in 3.8 has reproducible steps The issue has been confirmed reproducible and is ready to work on P2 Important issues not at the top of the work list platform-windows Building on or for Windows specifically team-engine Owned by Engine team triaged-engine Triaged by Engine team
Projects
None yet
Development

No branches or pull requests

7 participants