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

Ports shared through IsolateNameServer have unexpected behavior #147469

Closed
iazel opened this issue Apr 27, 2024 · 9 comments
Closed

Ports shared through IsolateNameServer have unexpected behavior #147469

iazel opened this issue Apr 27, 2024 · 9 comments
Labels
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.19 Found to occur in 3.19 found in release: 3.22 Found to occur in 3.22 has reproducible steps The issue has been confirmed reproducible and is ready to work on P1 High-priority issues at the top of the work list r: fixed Issue is closed as already fixed in a newer version team-engine Owned by Engine team triaged-engine Triaged by Engine team

Comments

@iazel
Copy link

iazel commented Apr 27, 2024

Steps to reproduce

  1. Register a port through IsolateNameServer.registerPortWithName
  2. Either don't listen on the port, or close the port, or Isolate.kill() the given isolate
  3. Retrieve the port through IsolateNameServer.lookupPortByName
  4. Send a custom class through send

Please be aware that it is important to send a custom class, otherwise the issue does not exist.

Another interesting aspect is that if we get port before it is closed, and keep using it, then no issue arise.

Expected results

Ports normally exhibit the following behavior:

  • Messages sent to a port that isn't ready (i.e. not listening), will be delivered as soon as listen is called
  • Sending messages to a closed port is a no-op
  • Data can be sent according to documentation, including the same-code ones

Actual results

When using data types supported across different code-bases (e.g: int, string, etc...), no issues arise.

When using data types supported only on same code-bases (e.g. custom classes), then ArgumentError is thrown on port.send if the receiving end is somehow closed or not ready.

Code sample

Code sample
import 'dart:isolate';
import 'dart:ui';

import 'package:flutter_test/flutter_test.dart';

final class A {
}

void main() {
  test('isolate', () async {
    TestWidgetsFlutterBinding.ensureInitialized();
    await Isolate.spawn(
      (_) {
        final port = ReceivePort();
        port.listen((_) {
          Isolate.exit();
        });
        IsolateNameServer.registerPortWithName(port.sendPort, 'test');
      },
      null,
    );

    while (IsolateNameServer.lookupPortByName('test') == null) {
      await Future<void>.delayed(const Duration(milliseconds: 100));
    }

    for (var i = 0; i < 10; ++i) {
      IsolateNameServer.lookupPortByName('test')?.send(A());
      await Future<void>.delayed(const Duration(milliseconds: 100));
    }
  });
}

Screenshots or Video

No response

Logs

Logs
dart:isolate                  _SendPort.send
test/isolate_test.dart 28:51  main.<fn>

Invalid argument: is a regular instance reachable via : Instance of 'A'

Flutter Doctor output

Doctor output
Doctor summary (to see all details, run flutter doctor -v):
[✓] Flutter (Channel stable, 3.19.6, on Ubuntu 23.10 6.5.0-28-generic, locale en_US.UTF-8)
[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
[✓] Chrome - develop for the web
[✓] Linux toolchain - develop for Linux desktop
[✓] Android Studio (version 2023.2)
[✓] IntelliJ IDEA Community Edition (version 2023.3)
[✓] Connected device (2 available)
[✓] Network resources

• No issues found!
@iazel
Copy link
Author

iazel commented Apr 27, 2024

After further analysis on Android, in case the main Activity is destroyed (i.e. AppLifecycleState.detached), even though the Isolate still runs in background, once we open back the app, port will be limited on sending data as if they didn't share the same codebase.

Notice that even if we get paused, the app will work without problems. It is only the detached state that causes problems.

This can be easily tested on Android by enabling Don't keep activities in Developer options settings.

Code sample:

import 'dart:async';
import 'dart:isolate';
import 'dart:ui';

import 'package:flutter/material.dart';

void main() {
    runApp(MyApp());
}

final class A {}

final class MyApp extends StatefulWidget {
  const MyApp({super.key});

  @override
  State<StatefulWidget> createState() {
    return _MyAppState();
  }
}

final class _MyAppState extends State<MyApp> {
  @override
  void initState() {
    super.initState();
    final port = IsolateNameServer.lookupPortByName('name');
    if (port == null) {
      Isolate.spawn((_) async {
        final port = ReceivePort();
        port.listen((message) {
          print('got $message');
        });
        IsolateNameServer.registerPortWithName(port.sendPort, 'name');
        var i = 0;
        while (true) {
          ++i;
          await Future<void>.delayed(const Duration(seconds: 5));
          print('looped $i times');
        }
      }, null);
    } else {
      port.send(A());
    }
  }

  @override
  Widget build(BuildContext context) {
    return const MaterialApp(
      home: Scaffold(
        body: Text('Hello World'),
      ),
    );
  }
}

By changing send to something like:

port.send('restarted');

It will work as expected.

@huycozy huycozy added the in triage Presently being triaged by the triage team label Apr 29, 2024
@huycozy
Copy link
Member

huycozy commented Apr 29, 2024

Thanks for the report. I also see the issue with the test and Don't keep activities on app as reported. This doesn't seem to belong to Android as I see the same test failed on macOS. Labeling the issue for engine team input.

flutter doctor -v (stable and master)
[✓] Flutter (Channel stable, 3.19.6, on macOS 14.1 23B74 darwin-x64, locale en-VN)
    • Flutter version 3.19.6 on channel stable at /Users/huynq/Documents/GitHub/flutter
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 54e66469a9 (31 hours ago), 2024-04-17 13:08:03 -0700
    • Engine revision c4cd48e186
    • Dart version 3.3.4
    • DevTools version 2.31.1

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/huynq/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • ANDROID_HOME = /Users/huynq/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.3)
    • Xcode at /Applications/Xcode15.3.app/Contents/Developer
    • Build 15E204a
    • CocoaPods version 1.15.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2023.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • 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
    • android-studio-dir = /Applications/Android Studio.app/
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)

[✓] VS Code (version 1.88.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.86.0

[✓] Connected device (3 available)
    • RMX2001 (mobile) • EUYTFEUSQSRGDA6D • android-arm64  • Android 11 (API 30)
    • macOS (desktop)  • macos            • darwin-x64     • macOS 14.1 23B74 darwin-x64
    • Chrome (web)     • chrome           • web-javascript • Google Chrome 123.0.6312.124

[✓] Network resources
    • All expected network resources are available.

• No issues found!
[!] Flutter (Channel master, 3.22.0-18.0.pre.53, on macOS 14.1 23B74 darwin-x64, locale en-VN)
    • Flutter version 3.22.0-18.0.pre.53 on channel master at /Users/huynq/Documents/GitHub/flutter_master
    ! Warning: `flutter` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/flutter, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path.
    ! Warning: `dart` on your path resolves to /Users/huynq/Documents/GitHub/flutter/bin/dart, which is not inside your current Flutter SDK checkout at /Users/huynq/Documents/GitHub/flutter_master. Consider adding /Users/huynq/Documents/GitHub/flutter_master/bin to the front of your path.
    • Upstream repository https://github.com/flutter/flutter.git
    • Framework revision 098e7e7ff3 (2 hours ago), 2024-04-29 01:25:19 +0000
    • Engine revision 752b146df7
    • Dart version 3.5.0 (build 3.5.0-109.0.dev)
    • DevTools version 2.35.0-dev.16
    • If those were intentional, you can disregard the above warnings; however it is recommended to use "git" directly to perform update checks and upgrades.

[✓] Android toolchain - develop for Android devices (Android SDK version 34.0.0)
    • Android SDK at /Users/huynq/Library/Android/sdk
    • Platform android-34, build-tools 34.0.0
    • ANDROID_HOME = /Users/huynq/Library/Android/sdk
    • Java binary at: /Applications/Android Studio.app/Contents/jbr/Contents/Home/bin/java
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)
    • All Android licenses accepted.

[✓] Xcode - develop for iOS and macOS (Xcode 15.3)
    • Xcode at /Applications/Xcode15.3.app/Contents/Developer
    • Build 15E204a
    • CocoaPods version 1.15.2

[✓] Chrome - develop for the web
    • Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome

[✓] Android Studio (version 2023.2)
    • Android Studio at /Applications/Android Studio.app/Contents
    • 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
    • android-studio-dir = /Applications/Android Studio.app/
    • Java version OpenJDK Runtime Environment (build 17.0.9+0-17.0.9b1087.7-11185874)

[✓] VS Code (version 1.88.1)
    • VS Code at /Applications/Visual Studio Code.app/Contents
    • Flutter extension version 3.86.0

[✓] Connected device (2 available)
    • macOS (desktop) • macos  • darwin-x64     • macOS 14.1 23B74 darwin-x64
    • Chrome (web)    • chrome • web-javascript • Google Chrome 124.0.6367.92

[✓] Network resources
    • All expected network resources are available.

! Doctor found issues in 1 category.

@huycozy huycozy added engine flutter/engine repository. See also e: labels. has reproducible steps The issue has been confirmed reproducible and is ready to work on team-engine Owned by Engine team found in release: 3.19 Found to occur in 3.19 found in release: 3.22 Found to occur in 3.22 and removed in triage Presently being triaged by the triage team labels Apr 29, 2024
@a-siva
Copy link
Contributor

a-siva commented Apr 29, 2024

//cc @aam

@jonahwilliams jonahwilliams added dependency: dart Dart team may need to help us P1 High-priority issues at the top of the work list triaged-engine Triaged by Engine team labels Apr 29, 2024
@aam
Copy link
Member

aam commented Apr 29, 2024

The issue with original code #147469 (comment) is that registered isolate exits once it receives first message, so no further sends will work. It's confusing of course that the error talks about invalid object and only shows up if it's an instance of user-defined class being sent, it silently does nothing if primitive object is being sent.

@iazel
Copy link
Author

iazel commented Apr 30, 2024

The issue with original code #147469 (comment) is that registered isolate exits once it receives first message, so no further sends will work. It's confusing of course that the error talks about invalid object and only shows up if it's an instance of user-defined class being sent, it silently does nothing if primitive object is being sent.

Yes, we exit the isolate just to showcase the diverging behaviour. Is it expected for send to fail in case the port is closed/not available?

@aam
Copy link
Member

aam commented Apr 30, 2024

Is it expected for send to fail in case the port is closed/not available?

No, it's not. Send should not behave differently whether port is open or closed. The underlying root cause of the exception is gap in the dart vm api Dart_NewSendPort/Dart_SendPortGetId that IsolateNameServer uses to keep track of opened send ports.

@a-siva a-siva added the dependency:dart-triaged Triaged by Dart team label May 1, 2024
aam added a commit to flutter/engine that referenced this issue May 2, 2024
…52498)

Currently used Dart API can not be reliably used to rebuild dart
SendPort object, [new API was introduced in dart sdk
](https://dart.googlesource.com/sdk/+/de4029ee1ba3bc89d839ae7d2fd0706d29abae58)
and this PR moves flutter engine to use that new API.

BUG: flutter/flutter#147469
@aam
Copy link
Member

aam commented May 4, 2024

The fix should have been rolled in flutter at 21da3a8, main channel at this point

@aam aam closed this as completed May 4, 2024
@huycozy huycozy added the r: fixed Issue is closed as already fixed in a newer version label May 6, 2024
@iazel
Copy link
Author

iazel commented May 7, 2024

@aam do you know if the fix will also solve #113825 and #119589 ?

@aam
Copy link
Member

aam commented May 8, 2024

@aam do you know if the fix will also solve #113825 and #119589 ?

I don't think those issues are related to this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.19 Found to occur in 3.19 found in release: 3.22 Found to occur in 3.22 has reproducible steps The issue has been confirmed reproducible and is ready to work on P1 High-priority issues at the top of the work list r: fixed Issue is closed as already fixed in a newer version team-engine Owned by Engine team triaged-engine Triaged by Engine team
Projects
None yet
Development

No branches or pull requests

5 participants