Skip to content
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

Synchronous I/O functions from dart:io don't prevent an isolate group from hanging #57119

Closed
abitofevrything opened this issue Nov 18, 2024 · 3 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-io triaged Issue has been triaged by sub team type-enhancement A request for a change that isn't a bug

Comments

@abitofevrything
Copy link
Contributor

abitofevrything commented Nov 18, 2024

This issue is in spirit a continuation of #51254; I'm just picking up where that issue left off.

I would have assumed that since that issue was closed and Dart_EnterIsolate/Dart_ExitIsolate are now part of the public API, dart:io's sync functions would have been updated to call these methods themselves. However this doesn't seem to be the case, and calling sync functions in many isolates at once can still cause an isolate group to hang.

Example:

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

void main() async {
  print('Main isolate alive!');

  Timer.periodic(
    Duration(milliseconds: 100),
    (_) => print('Main isolate alive!'),
  );

  // 16 is the limit, as documented in #51254. Setting this to 15 allows the main isolate to keep running
  for (int i = 0; i < 16; i++) {
    Isolate.spawn((_) {
      while (true) {
        print('Isolate $i alive');

        // Any synchronous function call that blocks the isolate. Here are some examples:
        // Process.runSync('sleep', ['1']);
        // File.fromUri(Platform.script).readAsBytesSync();
        // Directory.current.listSync();
        // Directory.current.createSync(recursive: true);
        sleep(Duration(seconds: 1));
      }
    }, null);
  }
}

I would expect these functions to call Dart_ExitIsolate (or some internal equivalent) before running the blocking call (whether it be poll in the case of Process.runSync, sleep in the case of sleep or any other blocking native call) in order to allow other isolates in the isolate group to run. Maybe this could also be done when calling into FFI isLeaf: true calls?

I don't know of any other blocking functions in the SDK other than dart:io's, but if there are some it goes without saying the same should be done there.

Tested on dart version 3.7.0-132.0.dev and 73adec7

@dart-github-bot
Copy link
Collaborator

Summary: Synchronous dart:io functions don't release the isolate group, causing hangs when many isolates use them concurrently. The issue persists despite Dart_EnterIsolate/Dart_ExitIsolate being public.

@dart-github-bot dart-github-bot added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) labels Nov 18, 2024
@abitofevrything
Copy link
Contributor Author

Maybe the overhead of entering/exiting the isolate isn't worth it for some very quick calls - but I can definitely see it being useful on longer calls (notably Process.runSync, which is where we noticed this issue).

@lrhn lrhn added library-io type-enhancement A request for a change that isn't a bug and removed type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Nov 20, 2024
@a-siva
Copy link
Contributor

a-siva commented Dec 4, 2024

This issue should get addressed as part of #54687

As a stopgap measure we can wrap Process.runSync and sleep with Dart_ExitIsolate and Dart_EnterIsolate with a comment to remove it when #54687 is addressed.

@a-siva a-siva added the triaged Issue has been triaged by sub team label Dec 4, 2024
copybara-service bot pushed a commit that referenced this issue Dec 5, 2024
This prevents such an isolate from occupying one of the limited number of mutator slots and blocking other isolates in the same group from running.

TEST=ci
Bug: #51254
Bug: #54687
Bug: #57119
Change-Id: Ic04bbaa7f482d533ad0ecf2c6da17ea9f00c264e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/398927
Reviewed-by: Alexander Aprelev <[email protected]>
Commit-Queue: Ryan Macnak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. library-io triaged Issue has been triaged by sub team type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants