How to make Isolate.spawn() safe

[mkustermann]

Right now Isolate.spawn() is implemented by calling out to the embedder, which supplies a Dart_IsolateCreateCallback during Dart_Initialize.

The VM has currently the assumption that isolates spawned via Isolate.spawn() have the exact same source code and the same representation of source code (e.g. snapshot, ...) because message sending internally relies on the isolates to have the same class id numbering.

Unfortunately this is not safe because:

• There is no guarantee that the embedder does load the same sources in the same representation inside the create callback.
• Even if the embedder manages to guarantee that via some mechanism, it still is flawed if we perform hot reload of one isolate (since this is not done for all isolates transitively spawned by the main isolate)

Is there a need for the embedder to implement Isolate.spawn() - why can't the VM do that internally (by re-using the same kernel/jit/aot snapshot used for the original isolate)?

If the only reason is to enable the embedder to disallow isolate spawning we could have a callback to the embedder for asking permission to spawn but the implementation for spawning can be inside the VM.

@rmacnak-google Do you see any reason why we could not change this?

Related issues where users have already ran into this:

• "JSON-RPC error 105: Isolate must be runnable" during hot reload causes tool to crash flutter/flutter#26568
• Flutter app spawns with two isolates, hot reload fails with Error 105: Isolate must be runnable flutter/flutter#26953
• Hot reloaded changes are not applied to new isolates as they are launched flutter/flutter#18600
• Hot Reload does not update background isolate flutter/flutter#20743
• Flutter crashes when opening a compute / isolate on iOS flutter/flutter#72195

[a-siva]

The standalone VM was changed to not use the source code but rather spawn from the kernel files that were used for spawning the parent isolate (see #6610 and the CL https://dart-review.googlesource.com/c/sdk/+/85724).

The Isolate reload case is still broken because we don't transitively reload all the isolates but at the minimum we should revert this hot reloaded isolate to behave similar to a spawnUri isolate and allow it to only send/receive messages of primitives or collections of primitives.

We have another effort going on to remove spawning of the Service and Kernel isolates from the VM and instead move them out to the embedder, once that is done we can take a look at cleaning up this create callback hook for the spawn case.

[rmacnak-google]

A few reasons the embedder needs to be involved in isolate spawning:

The embedder is needed to initialize the native half of core libraries like dart:io and dart:ui. The set of core libraries depends on the embedder.

The embedder is needed to decide which thread the isolate will run on. Before the isolate is made runnable, the embedder may use Dart_SetMessageNotifyCallback to prevent running the isolate on the VM's thread pool. The native methods in core libraries may require running on particular threads. dart:ui requires a thread with Skia resources. dart:zircon requires a thread with a libasync dispatcher. The isolate needs to move to such a thread before running these natives.

The embedder may want to enforce some resource limits. The flutter_runner and dart_runner embedders on Fuchsia run independent "components" in the same process. These embedders might try to set total max heap capacities, etc to prevent one component from monopolizing resources.

However, I don't think the embedder needs to be involved in loading the Dart code into the isolate. It seems reasonable to me that the VM would do this to guarantee isolates share code, though this probably requires new callbacks with the embedder.

[mkustermann]

Thanks for the quick feedback!

We have another effort going on to remove spawning of the Service and Kernel isolates from the VM
and instead move them out to the embedder

Is someone actively working on this? If so, please let me know who, so we can coordinate. Though I believe this is mostly orthogonal to what I want to look into.

However, I don't think the embedder needs to be involved in loading the Dart code into the isolate. It
seems reasonable to me that the VM would do this to guarantee isolates share code, though this
probably requires new callbacks with the embedder.

That was precisely my thinking as well. I would assume that two additional callbacks would suffice:

Similar to the current create callback we would add another create_spawned callback which will get called with the code already loaded into the isolate. The embedder can then do the remaining setup.

Though since multiple isolates share the same kernel/jit/aot snapshot buffers, another callback, e.g. free_isolate_buffers, could be called once all the isolates from the same source have died, so the underling resources can be freed (e.g. for calling munmap ...).

This would simplify the embedder code by removing the need to examine flags->copy_parent_code (plus the necessary code for loading same source) and allows the VM to have the same-source guarantee, do hot reload on all of those isolates as well as pave the way for the proposed lightweight isolates (which we are starting to work on).

These embedders might try to set total max heap capacities, etc to prevent one component from
monopolizing resources.

I suppose such resource constraints would apply for all of the component's isolates together (e.g. their sum cannot exceed X MB ram, Y % CPU ...) rather than per isolate?

[mkustermann]

As part of #36097 - which enabled isolate groups by-default now, hot-reload of all isolates within a group should be safe (modulo other non-isolate related issues).

Closing this.