dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.22k stars 1.57k forks source link

DDC and dart2js have different semantics for awaiting a Future which runs `.then` callbacks in a different Zone. #50619

Open natebosch opened 1 year ago

natebosch commented 1 year ago

cc @lrhn for help with what the expected semantics are for awaiting a Future that behaves in this way.

A typical Future will run the callback pass to .then in whatever Zone the .then call was made in. Angular has an implementation which overrides .then to run _innerFuture.then in a different zone. On the VM and in dart2js, when you await one of these futures the method resumes in the zone that has the _innerFuture.then call, whereas in DDC when you await one of these future the method resumes in the same zone that it started in.

With the following code, both the VM and dart2js will print:

Main: true
Before await: false
After await: true

While DDC will print:

Main: true
Before await: false
After await: false
import 'dart:async';

bool get inRoot => Zone.current == Zone.root;

void main() {
  print('Main: $inRoot');
  var f = ZonedFuture(Future.value(0), Zone.current.run);
  runZoned(() {
    doSomething(f);
  });
}

Future<void> doSomething(Future<void> f) async {
  print('Before await: $inRoot');
  await f;
  print('After await: $inRoot');
}

class ZonedFuture<T> implements Future<T> {
  final Future<T> _innerFuture;
  final T Function<T>(T Function()) _runInZone;

  ZonedFuture(this._innerFuture, this._runInZone);

  @override
  Stream<T> asStream() {
    return _innerFuture.asStream();
  }

  @override
  Future<T> catchError(Function onError, {bool Function(Object error)? test}) {
    return _runInZone(() => _innerFuture.catchError(onError, test: test));
  }

  @override
  Future<S> then<S>(FutureOr<S> Function(T value) onValue,
      {Function? onError}) {
    return _runInZone(() => _innerFuture.then<S>(onValue, onError: onError));
  }

  @override
  Future<T> timeout(Duration timeLimit, {FutureOr<T> Function()? onTimeout}) {
    return _runInZone(() {
      return _innerFuture.timeout(timeLimit, onTimeout: onTimeout);
    });
  }

  @override
  Future<T> whenComplete(FutureOr Function() action) {
    return _runInZone(() => _innerFuture.whenComplete(action));
  }
}
lrhn commented 1 year ago

Both implementations are within the very loose specification of await.

The behavior of await is defined in wibbly-wobbly terms like "when the future completes, use its value to ..." which doesn't actually say what happens. It should probably be defined in terms of calling Future.then, because then there'd be some guideline to how it actually behaves, and that's very likely what we will do for any non-natiive-_Future future. (We could use catchError or whenComplete to implement async try/catch/finally, but I don't think we do.)

There is no mention of zones in the language specification, so the zone-behavior of futures is entirely documented by the Future API, and the language specification doesn't say how it uses that API.

It's unspecified, basically.

The Future API does say something about zones. A Future should ensure that its callbacks (of then, catchError and whenComplete) are called in the zone that is current at the time of the then call. That's what ensures that the entire asynchronous computation is contained in that zone. It's why zones exist.

The implementations do need to call some members of Future. We have internal methods on _Future to help await implementations, but that won't do anything for awaiting a ZonedFuture. I'm pretty sure we always call .then.

When implementing await, there are two things you can do to make that the callbacks to .then of a non-_Future future continue in the same zone as the current code:

I'm guessing dart2js' await implementation is taking the "paranoid" approach, but for non-paranoid reasons. It's probably more performant, or just easier, to control zones manually and then go through non-public members on _Future which don't do the zone management for you. Then they're likely using some of the same code for non-_Future futures too. (Possibly with a slight overhead, that isn't normally necessary, but which saves the day here.)

And DDC takes the "nothing" approach, because it knows that code is running in the correct zone, so it can just make Future.then handle the zones.

When someone then implements a Future which breaks the documented behavior, they risk getting what they asked for. In this case, the code after an await is the callback passed to then, so it runs in the zone the Future makes it run in. That's pretty much expected behavior for such a future, it's what would happen if you called .then manually.

The problem is ZonedFuture. If you want to run code in a different zone, bind the code to that zone, don't mess with the Future. Not if you can't control where the Future flows, or who calls .then with which assumptions.

What ZonedFuture does is non-standard, non-API-conformant behavior. You can get away with that if you control the value, so it doesn't flow into code that you haven't written yourself. The second such a future flows into other code, like Future.wait or the implementation of await, you cannot predict how the non-standard behavior will interact with that other code. You might be lucky, and it "just works". Or you might end up with a future which never completes, because it ends up putting an error into a different error zone.