dart-lang / native

Dart packages related to FFI and native assets bundling.
BSD 3-Clause "New" or "Revised" License
157 stars 45 forks source link

Possibility of using `Looper.loop()` from the Dart thread? #1449

Open kekland opened 3 months ago

kekland commented 3 months ago

Hi!

I'm currently trying out jnigen to set up a video player. I decided to use the latest media3.ExoPlayer as the player, and was able to generate all of the bindings in Dart, which is awesome.

However, when playing around with ExoPlayer I ran into threading issues:

_player = ExoPlayer_Builder(context).build();
_player.play(); // <- Throws an exception saying that the access happened in a wrong thread

I was able to get around this by setting the looper for ExoPlayer_Builder to the looper of the Dart's thread (and preparing it beforehand):

Looper.prepare();

_player = ExoPlayer_Builder(context).setLooper(Looper.myLooper()).build();
_player.play(); // <- All good now

However, when adding callback listeners to the player, no callbacks actually go through. This is because the Looper.loop() was not called anywhere.

I can't call .loop() from any of the methods that can be accessed from the UI, as it would hang the thread. I tried calling it as:

Looper.prepare();

Future.delayed(Duration.zero, () async {
  Looper.loop();
});

This does work - all callbacks are sent through, but causes the app to crash when hot-restarting, because of an assertion when calling Dart_EnterIsolate.

I also tried calling the .loop() method from a different Isolate, but that didn't work, because the other Isolate has a different thread.

Attaching the player to the main looper does work, but the problem there is that I have to wrap all of my calls with ui.runOnPlatformThread(), and that the closure there seems to overcapture the context, so it's extremely unwieldy to use :(

Not sure how I can get the looper to work. I'm probably doing something stupid because I'm not very well-versed with JNI or the Android platform in general. Any help or ideas would be appreciated

HosseinYousefi commented 3 months ago

Hi,

Thanks for trying JNIgen out! The "official" way is to use ui.runOnPlatformThread(). I have not tried to use Looper, can you send me a small example repo so I can jumpstart my exploration on why the crash happens?

kekland commented 3 months ago

Hi! Sure, will prepare an example now

kekland commented 3 months ago

https://github.com/kekland/jnigen_looper_example

I've added some of the crashes that I encountered and the associated reproduction steps / stack traces in the README file.

HosseinYousefi commented 3 months ago

If the problem is with hot restart, maybe you also want a hook for it: https://github.com/flutter/flutter/issues/75528, https://github.com/dart-lang/sdk/issues/42679

cc @dcharkes

kekland commented 3 months ago

Actually no, we can't really quit the looper. Quitting it is a one-time only operation, and the assumption is that once it's quitted, you can no longer restart it. Therefore, the Looper.loop() itself has to be placed somewhere outside of the isolate.

Currently, trying to hot restart with an active looper results in the following crash:

Abort message: '../../../flutter/third_party/dart/runtime/vm/dart_api_impl.cc: 1534: error: Dart_EnterIsolate expects there to be no current isolate. Did you forget to call Dart_ExitIsolate?'

My guess - since the looper never exits, the isolate can't really be exited.

^ This is based on the fact that currently hot-restarting the app keeps the same thread (the Looper is created for the ui thread; printing it after hot-restarting shows the same thread id).

HosseinYousefi commented 3 months ago

I see, I checked out the documentation for Looper and it seems like it's attached to a thread. However in general there is no guarantee that the main isolate will always use the same thread.

There is ongoing work to merge the Flutter's UI and platform threads in the future. This is already landed for iOS on the main branch, not yet for Android so you won't need to use runOnPlatformThread anymore.

Looper is a final class but still it would be cool if we can have an implementation of it – say FlutterLooper that schedules tasks on the message queue of Flutter. Especially if there are other APIs that allow you to setLooper.

kekland commented 3 months ago

There is ongoing work to merge the Flutter's UI and platform threads in the future. This is already landed for iOS on the main branch, not yet for Android so you won't need to use runOnPlatformThread anymore.

Didn't know about this, thanks! This sounds really interesting. I think it was merged already for Android and iOS behind an experimental flag? (https://github.com/flutter/engine/commit/67bf5811b32b0870b250c630d1f31d5805eb2275)

I just tried it out on Android, and it seems like it works, calling Looper.myLooper() from Dart returns the main looper. However, I can't actually post any runnable to a handler for that looper, the app just hangs. My guess is that it has something to do with how interfaces are implemented from the Dart side (didn't have time to debug, maybe can try it out tomorrow).

Looper is a final class but still it would be cool if we can have an implementation of it – say FlutterLooper that schedules tasks on the message queue of Flutter. Especially if there are other APIs that allow you to setLooper.

Yeah, that would be awesome.

I think as it stands the only way to get it working with support for hot-restart would be to invoke the Looper.loop() method when the thread is created, so somewhere in the engine I guess. But I don't think it's good, since not everyone needs an active looper. Also, there's the issue of the looper's messages being executed asynchronously, and if the isolate gets killed while the looper is active, then some messages might be executed when the isolate is dead or a new one is already active, leading to some very bad stuff I guess. So the scope of this is way larger than I first anticipated.