Make send-and-exit a public API

[mkustermann]

Isolate groups were enabled by-default in the September beta (though it's still possible to disable it via the flag).

In the October or November beta we're going to remove the flag entirely and make it the only option (if we don't hear anything concerning until then). As part of that we should make send-and-exit a public API.

@aam Could you work with @lrhn on this?

[mit-mit]

I'd love to see this API in the September beta (which goes out in October) so we could potentially get some feedback prior to going to stable. Is that possible? Tagged for now, let me know if we should change.

[lrhn]

I'll start brainstorming.

As I understand it, sendAndExit will take a native SendPort and an object to send, and then terminate the isolate immediately after sending.

It's basically:

/// Sends [message] to [port] and synchronously terminates the current isolate.
static Never sendAndExit(SendPort port, Object? value) {
  port.send(value);  // If port isn't a native send-port
  Isolate.exit(); // Synchronously kills the current isolate, like `Process.exit` would for the process.
}

I'd make it static method, not an instance method on SendPort, both since it's not something you can implement on your own class that implements SendPort, and because it acts on the isolate at least as much as on the send port.

With that, we should also add Isolate.exit as a primitive, because even if we don't, you can implement it yourself as:

  static Never killCurrentIsolate() {
    sendAndExit(RawReceivePort().sendPort, null);
  }

So, maybe we should instead make Isolate.exit be the basic operation, and give it an optional "final words" argument:

class Isolate {
  ...
  /// Terminates the current isolate synchronously.
  ///
  /// This operations is potentially dangerous and should be used judiciously.
  /// The isolate stops operating *immediately*. A call to this message does not return and
  /// also doesn't throw. Pending `finally` blocks are not executed,
  /// control flow will not go back to the event loop,
  /// scheduled asynchronous asks will never run,
  /// and even pending isolate control commands may be ignored.
  /// (The isolate will send messages to ports already registered using [Isolate.addOnExitListener], 
  /// but no further Dart code will run in the isolate.)
  ///
  /// If [finalMessagePort] is provided, the [message] is sent to that port
  /// as the final operation of the current isolate.
  /// (If the port is a native port, one provided by [ReceiverPort.sendPort] or [RawReceivePort.sendPort],
  /// the system may be able to send this final message more efficiently than normal port communication
  /// between live isolates.)
  external static Never exit([SendPort finalMessagePort, Object? message]);

So, basically, the operation is "exit (and maybe send)" instead of "send and exit".

Since exiting is the one guaranteed side-effect, and sending can go wrong for any number of reasons (like using a send port linked to an already closed receive port), making exit be the primary part of the name and description makes sense.

So, that's my initial API suggestion: Isolate.exit(messagePort, "It's dead, Jim.");.
I think it reads well, as a kind of "isolate exit and return this value".

(We can't make the message required if finalMessagePort is provided, which is sad, because sending null is probably not that useful.)

Considerations:
Q. What if you want to send more than one message?
A. Then send a list. Isolate communication is always between cooperating entities who can agree on a protocol.

Q. What if I want to send the message to more than one port?
A. Then send it normally. We can't optimize that anyway. Sending is secondary to exiting in this API.

It's still incredibly dangerous to just drop all finally blocks and scheduled async operations. There is no knowing which clean-up won't happen because of that.

Another alternative could be to have exit instead schedule an asynchronous task, and not execute it until control comes back to the event loop (or microtask loop, maybe with an option to choose). I think we may sometimes want to complete all microtasks first, and only exit as the next operation of the top-level event loop.

Scheduling two exits will be an error (unless we provide a way to cancel it again, which may be useful. Imagine the horror of a program knowing that it has scheduled an exit, and desperately trying to keep coming up with microtasks, just to stay alive! Or don't anthropomorphise programs. Anyway ...).

Then the exit would essentially be equivalent to sending the Isolate.addOnExitListener message to yourself with the port and message, then follow it with an Isolate.kill control message (with a suitable priority), just without the copying of the message object that you'd normally get by sending through the command port,
and also marked as being sent by merging heaps when the on-exit response is sent.

That should be easily implementable. Should be named something like:

class Isolate {
  ...
  /// Schedules the isolate to terminate as soon as possible.
  ///
  /// Sets the current isolate to terminate as if by receiving an [Isolate.kill] command.
  /// Must only be called once.
  ///
  /// If [finalMessagePort] is provided, the [message] is sent to that port when the isolate has terminated,
  /// just like ports and messages registered using [Isolate.addOnExitListener].
  /// (If the port is a native port, one provided by [ReceiverPort.sendPort] or [RawReceivePort.sendPort],
  /// the system may be able to send this final message more efficiently than normal port communication
  /// between live isolates.)
  external static void scheduleExit([SendPort? finalMessagePort, Object? message]);
}

Piggy-backing on Isolate.kill does not allow for running the microtask queue. It's always required to terminate at the latest before the next event (which includes microtask events). With an "immediate" priority, it may be able to kill running code like Isolate.exit above, if the implementation allows it. Not sure we want that.

[aam]

Awesome brainstorm @lrhn !
Just wanted to note that sendAndExit can fail if you are trying to send some objects that we do not allow to send. So from this perspective it might be confusing to see that exit(message) fails in this fashion as your expectation might be that you are exiting first and foremost, sending message is secondary effect.

Only other consideration I have is that having sendAndExit go into effect immediately feels important because you want to be sure that object you are sending is what you have at hand when you call sendAndExit. Probably you don't want to have any tampering done to this object by async/finally code.

[dnfield]

related: #37835

[dnfield]

I would probably agree with @lrhn about finally blocks not running being problematic. For example, you might be using a finally to flush and close a file descriptor, and be surprised to find that sometimes your file doesn't end the way you expected (or get closed precisely when you thought it would).

[dnfield]

From the other thread, the concern about whether an object gets collected on a different thread probably isn't a huge deal - the VM already does that to us anyway, e.g. when running a multi-threaded collection I've observed the object finalizers getting run on a VM worker thread rather than the mutator thread.

[aam]

Another reason to have sendAndExit take effect right away without execution of the finally clauses is that that is what Isolate.kill is doing and one can argue that sendAndExit is effectively killing current isolate and sending some message to some designated SendPort.

[dnfield]

Perhaps sendAndKill would be a clearer name then?

Alternatively, if possible, have some kind of parameter to sendAndExit to control whether it's killed or allowed to finish anything.

[aam]

Perhaps sendAndKill would be a clearer name then?

We tried alternative along that line of thinking, but it was dismissed early on.

Alternatively, if possible, have some kind of parameter to sendAndExit to control whether it's killed or allowed to finish anything.

If at all possible I think it would be better to avoid parameters customizing this kind of behavior to keep things simpler for user.
In case of flutter compute for example, I think you would naturally have sendAndExit as a last statement in your top level compute function.

sendAndExit has immediate aspect, which is to verify whether message object can be actually sent, then there is potentially-delayed aspect of an isolate exiting and message passing. If we let dart user code run between these two points, message object might be updated so it no longer passes verification check for example.

[lrhn]

The word Exit matches Process.exit and therefore carries the connotation of immediately exiting. (And avoids the "kill"/"die" wording, which honestly should be avoided. Sorry about Isolate.kill, it should have been .shutdown or .exit instead).

As mentioned before, we can make it delay the send until being back at the event loop, but that only ensures evaluation of synchronous finally block, not async ones, so it's only a partially solution. Even then, I suggested returning a Future which would complete if sending failed, possibly with the error (and not if sending succeeded, because then the isolate would have shut down). That would also entirely delay serialization until that point, rather than doing double work.

[aam]

sendAndExit can fail without exiting due to having unsendable objects in the message.
Having it run synchronously without execution of the finally-blocks matches existing Isolate.kill.

If you are fine with inconsistency with Isolate.kill, yes we could have sendAndExit return a Future and complete it(with error) if the verification failed. To me sendAndExit returning a Future does not sound intuitive - it's not immediately obvious what we are waiting for - it's basically just a mechanism to run finally clauses.

[lrhn]

Agree, and as I mentioned, it only runs synchronous finally clauses, so it's only removing some of the risk of an early exit, not all of it. So, exiting immediately is probably fine.

[dnfield]

Just to be sure - Flutter's compute function would be fine here because how we set up the ports right?

See https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/foundation/_isolates_io.dart#L92

If we changed line 98 to use sendAndExit, and then removed the call to isolate.kill, all finally blocks within the callback would still execute (assuming some error did not result in early termination of the isolate)?

[aam]

@dnfield yes, Flutter's compute should be fine(back when we introduced sendAndExit internally we tested that too).
The callback should be done and all it's finally clauses should be completed by the time we get to sendAndExit(or Isolate.exit(port, object) as @lrhn proposed) at line 98.

[dnfield]

Great. In that case I'm fine with this. I think if anything we'd just add a comment in the implementation to remind ourselves that we have to make sure any important work is finished because anything that's unawaited or in a finally block will not execute after sendAndExit.

[lrhn]

(Flutter's compute does not seem to be adversely affected by this. It's not actually safe as written: There is no guarantee about the order of delivery of events sent to different receive-ports, even between the same two isolates. With the current code, It's technically possible (at least allowed, whether the implementation can do it or not) to send a result, terminate, send the onExit event, and have the onExit handler react before the result handler. With the current code, an error event could also arrive at any time, if the error happens after sending the result.
Consider using the same port to receive all the messages, using null for the onExit message, a one-element list with the result and a two-element list for errors. I think we do promise events sent to the same receive-port from the same isolate to be delivered in-order.

Oh, well, now that I looked at it, maybe try this alternative (completely untested, naturally):

Future<R> compute<Q, R>(FutureOr<R> Function(Q) callback, Q message, { String? debugLabel }) async{
  debugLabel ??= kReleaseMode ? 'compute' : callback.toString();
  final Flow flow = Flow.begin();
  Timeline.startSync('$debugLabel: start', flow: flow);
  final ReceivePort messagePort = ReceivePort();
  Timeline.finishSync();
  final Isolate isolateFuture = await Isolate.spawn<_IsolateConfiguration<Q, FutureOr<R>>>(
    _spawn,
    _IsolateConfiguration<Q, FutureOr<R>>(
      callback,
      message,
      messagePort.sendPort,
      debugLabel,
      flow.id,
    ),
    errorsAreFatal: true,
    onExit: messagePort.sendPort,
    onError: messagePort.sendPort,
  );
  var message = await messagePort.first; // Closes port after first message.
  Timeline.startSync('$debugLabel: end', flow: Flow.end(flow.id));
  if (message == null) {
    // An onExit handler message.
    Timeline.finishSync();
    throw Exception('Isolate exited without result or error.'));
  } 
  assert(message is List<dynamic>);
  var list = message as List<dynamic>;
  if (list.length == 1) {
    // A result message.
    var result = list[0] as R;
    isolate.kill(); 
    Timeline.finishSync();
    return result;
  }
  assert(list.length == 2);
  // An onError handler message.
  // (Consider using `RemoteError(list[0] as String, list[1] as String)` instead
  // That's what `Isolate.errors` uses for remote errors.)
  final Exception exception = Exception(list[0]);
  final StackTrace stack = StackTrace.fromString(list[1] as String);
  Timeline.finishSync();
  await Future<Never>.error(exception, stack);
}

@immutable
class _IsolateConfiguration<Q, R> {
  const _IsolateConfiguration(
    this.callback,
    this.message,
    this.resultPort,
    this.debugLabel,
    this.flowId,
  );
  final isolates.ComputeCallback<Q, R> callback;
  final Q message;
  final SendPort resultPort;
  final String debugLabel;
  final int flowId;

  FutureOr<R> apply() => callback(message);
}

Future<void> _spawn<Q, R>(_IsolateConfiguration<Q, FutureOr<R>> configuration) async {
  final R result = await Timeline.timeSync(
    configuration.debugLabel,
    configuration.apply,
    flow: Flow.step(configuration.flowId),
  );
  Timeline.timeSync(
    '${configuration.debugLabel}: returning result',
    () { configuration.resultPort.send(List<Object?>.filled(1, result)); },
    flow: Flow.step(configuration.flowId),
  );
}

)

[dnfield]

Filed flutter/flutter#90693

[mit-mit]

Hey @aam, can we please add an entry for this in the CHANGELOG.md?

[aam]

@mit-mit, it was done in https://dart.googlesource.com/sdk/+/48db681c662b56d5d6eccab93c7997bfeba8640a

[mit-mit]

Perfect, thanks! (and sorry my search skills failed me)