Skip to content

Add synchronous API to spawn a new isolate within an isolate group #44088

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

Closed
mkustermann opened this issue Nov 6, 2020 · 4 comments
Closed
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@mkustermann
Copy link
Member

Right now any embedder can call Dart code which can spawn an isolate. If --enable-isolate-groups is turned on, the embedder will get then a Dart_InitializeParams.initialize_isolate callback to initialize the new isolates global state.

Embedder 
  |-- Invoke Dart
       |--call Isolate.spawn()      // <-- Returns `Future<Isolate>`
            |--> Dart VM
                 |--call `initialize_isolate`
                   |--> Embedder

This is cumbersome for various reasons:

  • One of the calls appears to the caller as async, it returns a Future<Isolate> and it's hard for embedder to wait for the Dart_Isolate to be ready. It is possible, by letting the embedder callback that initializes the isolate talk e.g. via C++ Monitor back to the original embedder and give it the isolate.
  • It is quite slow to go through Dart to achieve this
  • It doesn't allow the embedder to easily distinguish normal helper isolates spawned via Isolate.spawn() (for which it wants the VM to run eventloop) from isolates where it likes custom control (e.g. multiple UI isolates, where it wants to install special message handler).
@mkustermann mkustermann added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Nov 6, 2020
@mkustermann mkustermann self-assigned this Nov 6, 2020
@gaaclarke
Copy link
Contributor

I think it's impossible to wrap up Isolate.spawn in C++ with the current API because there is no way to transform a symbol string to a function for the first argument in Dart without the reflection library, which isn't always available. I also don't see a way to lookup a function from the Dart C API either.

If that is true, there is no working around this feature (which is a blocker for me =).

dart-bot pushed a commit that referenced this issue Nov 10, 2020
…es in VM implementation of Isolate.spawn()

This is purely a refactoring of code - apart from now requiring
the `Dart_InitializeParams.initialize_isolate` iff --enable-isolate-groups
was passed.

Issue #44088
Issue #36097

Change-Id: I62f84ce9699a376c652d4ac6af0c28027fb872a8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170920
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Alexander Aprelev <[email protected]>
@mkustermann
Copy link
Member Author

mkustermann commented Nov 10, 2020

I think it's impossible to wrap up Isolate.spawn in C++ with the current API because there is no way to transform a symbol
string to a function for the first argument in Dart without the reflection library, which isn't always available. I also don't see a
way to lookup a function from the Dart C API either.

The notion of functions is really a VM-internal concept. The embedder as well as Dart code only deals with instances and closures (*1). So maybe what you would like to have is to obtain a closure for a top-level method?

The C-based API lets one call global functions / instance methods with Dart_Invoke(), lets one call closures via Dart_InvokeClosure() as well as lets one look up fields via Dart_Get(). Those primitives are enough to do almost anything (*2)

When one calls Isolate.spawn(<closure>, <msg>) one is really passing a closure as argument. Using the primitives above it's possible to get closures of top-level methods, e.g. by invoking a helper method that returns it. Afterwards the closure can be called via Dart_InvokeClosure().

(*1) If you have a Function fun in Dart it really means fun is a closure object.
(*2) One cannot use Dart_GetField() for tearing off static methods - it is only allowed for main methods inside the root library. This restriction could potentially be lifted.

@gaaclarke
Copy link
Contributor

Thanks, that makes sense. I might be running into the limitations you are mentioning. Using Dart_GetField I can get "main" in my main.dart, it can't seem to get top-level functions in a different library though.

I think for my testing launching main is fine for my testing. I know you told me other things won't work but I'm not quick enough to understand it without being blocked by it. It's a bit confusing to me because it looks like Flutter should support launching different entry points with this API but only root+main works in my experience (https://github.com/gaaclarke/engine/blob/e0d515369fac81e34c2d48f39c6ca766096add54/runtime/dart_isolate.cc#L531)

@mkustermann
Copy link
Member Author

It's a bit confusing to me because it looks like Flutter should support launching
different entry points with this API but only root+main works in my experience

One can easily work around that by making this:

@pramga('vm:entry-point')
Function get getEntryPointClosure => entryPoint;

void entryPoint(...) {...}

So instead of doing Dart_Get(<entryPoint>) + Dart_InvokeClosure(...) you would use Dart_Invoke(<getEntryPointClosure>) + Dart_InvokeClosure(...). If it's a big inconvenience we could lift the restriction that only main methods in root libraries can be torn off atm.

dart-bot pushed a commit that referenced this issue Nov 11, 2020
We have currently two ways to shut down isolates from the outside:

  * An embedder shuts down an embedder-owned isolate via
    `Dart_ShutdownIsolate()`.

  * An `Isolate.spawn()`ed isolate finishes the VM-owned message loop
    and VM shuts it down.

The second mechanism uses almost the same as the first, but slightly
different. This CL makes us change the second one to call our normal
`Dart_ShutdownIsolate()`.

Approved failures that got revealed by moving canonicalization check
to all isolate shutdowns, see
Issue #44141

Issue #36097
Issue #44088

Change-Id: I58af0e9ae73842545e21edad23c106ea59b176b3
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170984
Reviewed-by: Ryan Macnak <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>
dart-bot pushed a commit that referenced this issue Nov 12, 2020
When Dart invokes Isolate.spawn the Dart VM calls out to the embedder to
create an isolate (group) or initialize it (in case of lightweight
isolates).

  * If the embedder doesn't implement this functionality or fails to
    perform the creation/initialization, it will signal this failure
    via the return value it gives to the VM in the call back
    implementation (i.e. Dart_InitializeParams.create_group and
    Dart_InitializeParams.initialize_isolate}).

  * If the embeder sucessfully completed it's work, the VM owns the
    isolate after the embedder call returns. The VM is then responsible
    for running it.

It is an undocumented invariant atm that the embedder is responsible
to make the isolate runnable if it signals the VM that isolate was
sucessfully created/initialized.
  => If it does not do that, we have effectively an isolate leak.
  => Though right now seemingly all of our embedders to that correctly.

To avoid this unintentional isolate leak we make the isolate runnable
ourselves if embedder callback was successful but did not make it
runnable itself.

  => This also avoids unnecessary ceremony code in the embedders that
     look like this:

      Dart_ExitIsolate();
      Dart_IsolateMakeRunnable(child);
      Dart_EnterIsolate(child);

The implementation of `Dart_IsolateMakeRunnable()` had an untested and
unused code path that caused running the isolate (on a thread pool).
This is not documented and can lead to bugs because often the
embedders enter the isolate right after making it runnable (e.g. above
code sequence) - which would be racing with the spawned message handler
thread.

  => Furtunately our embedders are well behaved and make the isolate
     runnable during the callback.
  => As a safeguard we'll make `Dart_IsolateMakeRunnable()` return an
     error if it's called outside the callback (which we can detect by
     the presense of a spawn state)

Issue #44088
Issue #36097

TEST=Changes internal details of implementation which is already covered by tests.

Change-Id: Ieef316cc0807d99f2fcb1fbd56bbc9c41e904ba8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/170882
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Alexander Aprelev <[email protected]>
Reviewed-by: Ryan Macnak <[email protected]>
dart-bot pushed a commit that referenced this issue Nov 15, 2020
…/ into runtime/lib

The Isolate.spawn() is part of our dart:isolate library implementation,
it should therefore preferrably live under runtime/lib folder.

This CL does that as well as removing some special message handling
logic by using the new `Dart_RunLoopAsync()` C API, which results in
net removal of code.

As a nice side-effect it also makes isolate spawning slightly faster due
to avoiding extra safepoint/vm<->native transitions.

Issue #44088

TEST=Changes existing implementation, have existing test coverage for it.

Change-Id: I00607a1436946552bbe12e8e23062e8f743d4f11
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/171731
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Ryan Macnak <[email protected]>
Reviewed-by: Alexander Aprelev <[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.
Projects
None yet
Development

No branches or pull requests

2 participants