Dart2JS has incorrect semantics for yield in async*.

[saibotma]

This issue uses a Flutter example, because it was the easiest way to execute code on web for me.
It is not a Flutter issue, though.

Steps to reproduce:

1. Run the example on Web (dev build)
2. Notice that it prints "Mapped event value" then "Yielded value"
3. Run the example on iOS (or other platform, did not test on other platform, though)
4. Notice that it prints "Mapped event value" then "Received value value"

Isn't web behaviour the correct one, because it is consistent with

When the listener cancels (by invoking cancel() on the StreamSubscription object returned by the listen() method), then the next time the body reaches a yield statement, the yield instead acts as a return statement.

from https://dart.dev/articles/libraries/creating-streams

Dart Version: 3.19.1
Chrome Version: 122.0.6261.69 arm64

Example

import 'dart:async';

import 'package:flutter/material.dart';

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

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

  @override
  State<MyApp> createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  @override
  void initState() {
    super.initState();

    _getStream()
        .map((event) {
          print("Mapped event $event");
          return event;
        })
        .first
        .then((value) {
          print("Received value $value");
        });
  }

  Stream<String> _getStream() async* {
    yield await Future.delayed(const Duration(seconds: 2), () => "value");
    print("Yielded value");
    await Future.delayed(const Duration(hours: 1));
  }

  @override
  Widget build(BuildContext context) {
    return const Placeholder();
  }
}

[lrhn]

No need for importing Flutter, or waiting entire hours. The following has the same behavior.

import 'dart:async';

void main() {
  initState();
}

void initState() {
  _getStream()
      .map((event) {
        print("Mapped event $event");
        return event;
      })
      .first
      .then((value) {
        print("Received value $value");
      });
}

Stream<String> _getStream() async* {
  await Future.delayed(const Duration(seconds: 2));
  yield "value";
  print("Yielded value");
  await Future.delayed(const Duration(seconds: 3));
}

It does appear to be a bug. The VM bug for the same thing was #34775
I think it has been fixed, which is why you don't see it when running as native (shouldn't matter that it's iOS, just that it isn't web/JavaScript).

What happens is:

• The _getStream body reaches the yield, after evaluating an expression to "value". Doesn't matter what that expression is.
• Then it emits "value" on the stream.
• The map gets the value immediately, prints it, and passes it on.
• The first gets the value, then cancels its subscription. This returns a future, which it waits for before completing its own future.
• Control goes back to the _getStream() function body at the yield.
• BUG: The code at that point does not check if the stream has been paused or cancelled. It should.
• So control continues into the following code, which in your example printes "Yielded value" and awaits for an hour.
• When that hour is up, the function exits, the cancel future completes, and the first code completes its future with the result "value".
• Then "Received value: value" is printed.

The bug is that async* implementations should check for whether the stream subscription has been paused or cancelled before continuing from a yield or yield*. If the subscription has been paused, it should wait for a resume or cancel. If the subscription has been cancelled, or is cancelled while paused, the yieldshould perform areturn`.

A cancel on the subscription of an async*-based stream will always wait for the function body to exit, to ensure that all finally clean-ups and transitive cancels are properly executed. However, if control continues after the yield, even if the receiver has eagerly cancelled, then the cancel will happen at the next yield instead.
So the implementation should return at the yield where the listener did the cancel in direct response to the event, to ensure not doing more work than necessary.

You can still screw up a correct return by having a try/finally { await Future.delayed(Duration(hours: 1)); } around it, but then you're just asking for problems. (And we deliver!)

Another example of the same thing, exposing what first does internally:

void main() {
  var s = _getStream();
  var u = s.listen(null);
  u.onData((v) async {
    print("value: $v");
    await u.cancel();
    print("cancelled");
  });
}
 Stream<String> _getStream() async* {
  await null;
  print("pre-yield");
  yield "a";
  print("post-yield");
  for (var i = 0; i < 5; i++) {
    await Future.delayed(Duration(milliseconds: 500));
    print("..tick");
  }
  yield "try again"; // This exits!
  print("done");
}

The VM runs correctly, printing "cancelled" after "value: a", and nothing more. Compiling to JS prints the "post-yield" and all the ticks before printing "cancelled".

I think Dart2Wasm works correctly too, not sure how DDC fares (can never figure out how to run it locally).

The existing language test for this issue is (IIRC): language/async_star/async_star_cancel_test

[saibotma]

Thanks for the clarification and detailed explanation. Helps to understand the internals!

For my understanding: Is it correct to say, that cancelling in direct (synchronous) response to a yield is an exception to the statement in the documentation (below), because it does not return at the next yield, but immediately after the last/current yield?

When the listener cancels (by invoking cancel() on the StreamSubscription object returned by the listen() method), then the next time the body reaches a yield statement, the yield instead acts as a return statement.

[lrhn]

That, or the statement is being technically correct.
What is the next time the body reaches a yield statement, when it's currently executing a yield statement?

But it could be expressed more clearly. If cancelling in synchronous response to an event being delivered, the async* function should return from the current yield.

[biggs0125]

Dart2js transforms async* method bodies into a state machine where each piece of code between an async pause point is a different state. The fix here is to add a check at the beginning of each post-yield state to see if the associated stream is cancelled. If so we can jump to the exit state immediately.

I've put together a change implementing this: https://dart-review.googlesource.com/c/sdk/+/355040

[lrhn]

There is more to it than just exiting if cancelled.

• Handling pauses at the return too. (That seems to already be handled.)
• Handling finally blocks on the way out. A yield for a cancelled subscription works like return, which means it triggers finally blocks on the way out, and if those finally blocks do control flow, or awaits, that has to be handled too.

I wrote this about desugaring async* for dart2wasm recently: #52464 (comment)
Same things apply here.

(I though dart2js used JS generators for async, or was that DDC?)
A yield is not necessarily an async pause point. It can continue synchronously if the event is delivered synchronously, and the subscription wasn't paused.

That is:

• A yield should emit an event, and wait for it to be delivered (using a sync controller makes that trivial, just call add). If the subscription has been cancelled, throwing away the event counts as delivered. If the subscription is paused, wait for it to become unpaused, which means all pending events have been delivered (but that can be rolled into the following check).
• Then it should check whether the subscription is paused. If so, wait for it to resume or cancel.
• Then it should check if the subscription is cancelled. If so perform a return.

If the state machine can handle the pause before re-entering to the body, that's fine. It likely does need to go back into the body to handle the "return" on a cancel. Whether that's by having a secondary entry point state to use in case of cancel (next finally block), or by returning a boolean that says whether to return, and then do the equivalent of a return from inside the body.

If the CL does that, it should be fine.

[biggs0125]

I've landed this fix and previously failing language tests are now passing: https://dart-ci.firebaseapp.com/cl/355040/5

Though it's a bug in the JS runtime, you can workaround this by using a raw Stream/StreamController rather than async*/yield. Given this workaround and the fact that developers are unlikely to encounter this (the bug has existed for a long time), I don't think this warrants a cherrypick.

Closing out, thanks for the help here, @lrhn!