Make nullFuture be per-zone.

We introduced a `nullFuture` during the null-safety migration where
we changed some methods to no longer allow returning `null`,
and they therefore had to return a `Future`.
That affected timing, because returning `null` was processed
synchronously, and that change in timing made some tests fail.
Rather that fix the fragile tests, we made the function return
a recognizable future, a canonical `Future<Null>.value(null)`,
and then recognized it and took a synchronous path for it.

That caused other issues, because the future was created in the
root zone. (Well, originally, it was created in the first zone
which needed one, that was worse. Now it's created in the root zone.)
Some code tries to contain asynchrony inside a custom zone, and
then the get a `nullFuture` and calls `then` on it, and that
schedules a microtask in the root zone.
(It should probably have used the listener's zone, and not store
a zone in the future at all, but that's how it was first done,
and now people rely on that behavior too.)

This change creates a `null` future *per zone* (lazily initialized
when asked for). That should be sufficient because the code recognizing
a returned `null` future is generally running in the same zone,
but if any other code gets the `nullFuture`, it will be in the
expected zone for where it was requested.

This is a reland of commit a247b15

Change-Id: Ieec74d6f93c57175c357ec18889144635f5bdca6
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249490
Commit-Queue: Lasse Nielsen <[email protected]>
Reviewed-by: Erik Ernst <[email protected]>

Author: lrhn

pkg/front_end/testcases/nnbd/issue42546.dart.weak.outline.expect

@@ -28,19 +28,19 @@ static method main() → dynamic
 
 
 Extra constant evaluation status:
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:821:13 -> SymbolConstant(#catchError)
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:821:13 -> ListConstant(const <Type*>[])
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:821:13 -> SymbolConstant(#test)
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:872:13 -> SymbolConstant(#whenComplete)
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:872:13 -> ListConstant(const <Type*>[])
-Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:872:13 -> MapConstant(const <Symbol*, dynamic>{})
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:916:13 -> SymbolConstant(#timeout)
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:916:13 -> ListConstant(const <Type*>[])
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:916:13 -> SymbolConstant(#onTimeout)
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:770:13 -> SymbolConstant(#then)
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:770:13 -> SymbolConstant(#onError)
-Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:881:13 -> SymbolConstant(#asStream)
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:881:13 -> ListConstant(const <Type*>[])
-Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:881:13 -> ListConstant(const <dynamic>[])
-Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:881:13 -> MapConstant(const <Symbol*, dynamic>{})
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:814:13 -> SymbolConstant(#catchError)
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:814:13 -> ListConstant(const <Type*>[])
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:814:13 -> SymbolConstant(#test)
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:865:13 -> SymbolConstant(#whenComplete)
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:865:13 -> ListConstant(const <Type*>[])
+Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:865:13 -> MapConstant(const <Symbol*, dynamic>{})
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:909:13 -> SymbolConstant(#timeout)
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:909:13 -> ListConstant(const <Type*>[])
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:909:13 -> SymbolConstant(#onTimeout)
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:763:13 -> SymbolConstant(#then)
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:763:13 -> SymbolConstant(#onError)
+Evaluated: SymbolLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:874:13 -> SymbolConstant(#asStream)
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:874:13 -> ListConstant(const <Type*>[])
+Evaluated: ListLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:874:13 -> ListConstant(const <dynamic>[])
+Evaluated: MapLiteral @ org-dartlang-sdk:///sdk/lib/async/future.dart:874:13 -> MapConstant(const <Symbol*, dynamic>{})
 Extra constant evaluation: evaluated: 61, effectively constant: 15

sdk/lib/async/async.dart

@@ -109,6 +109,7 @@ import "dart:_internal"
         CastStreamTransformer,
         checkNotNullable,
         EmptyIterator,
+        isNullFuture,
         IterableElementError,
         nullFuture,
         printToZone,

sdk/lib/async/future.dart

@@ -223,13 +223,6 @@ abstract class FutureOr<T> {
 /// it's very clearly documented.
 @pragma("wasm:entry-point")
 abstract class Future<T> {
-  /// A `Future<Null>` completed with `null`.
-  ///
-  /// Currently shared with `dart:internal`.
-  /// If that future can be removed, then change this back to
-  /// `_Future<Null>.zoneValue(null, _rootZone);`
-  static final _Future<Null> _nullFuture = nullFuture as _Future<Null>;
-
   /// A `Future<bool>` completed with `false`.
   static final _Future<bool> _falseFuture =
       new _Future<bool>.zoneValue(false, _rootZone);

sdk/lib/async/stream.dart

@@ -515,7 +515,7 @@ abstract class Stream<T> {
       controller
         ..onCancel = () {
           timer.cancel();
-          return Future._nullFuture;
+          return nullFuture;
         }
         ..onPause = () {
           watch.stop();

sdk/lib/async/stream_controller.dart

@@ -588,7 +588,10 @@ abstract class _StreamController<T> implements _StreamControllerBase<T> {
   Future<void> get done => _ensureDoneFuture();
 
   Future<void> _ensureDoneFuture() =>
-      _doneFuture ??= _isCanceled ? Future._nullFuture : _Future<void>();
+      _doneFuture ??
+      (_isCanceled
+          ? nullFuture as Future<void>
+          : _doneFuture = _Future<void>());
 
   /// Send or enqueue a data event.
   void add(T value) {
@@ -919,7 +922,7 @@ class _AddStreamState<T> {
     var cancel = addSubscription.cancel();
     if (cancel == null) {
       addStreamFuture._asyncComplete(null);
-      return Future._nullFuture;
+      return nullFuture;
     }
     return cancel.whenComplete(() {
       addStreamFuture._asyncComplete(null);

sdk/lib/async/stream_impl.dart

@@ -197,7 +197,7 @@ class _BufferingStreamSubscription<T>
     if (!_isCanceled) {
       _cancel();
     }
-    return _cancelFuture ?? Future._nullFuture;
+    return _cancelFuture ?? nullFuture;
   }
 
   Future<E> asFuture<E>([E? futureValue]) {
@@ -217,7 +217,7 @@ class _BufferingStreamSubscription<T>
     };
     _onError = (Object error, StackTrace stackTrace) {
       Future cancelFuture = cancel();
-      if (!identical(cancelFuture, Future._nullFuture)) {
+      if (!isNullFuture(Zone._current, cancelFuture)) {
         cancelFuture.whenComplete(() {
           result._completeError(error, stackTrace);
         });
@@ -297,7 +297,7 @@ class _BufferingStreamSubscription<T>
   // Hooks called when the input is paused, unpaused or canceled.
   // These must not throw. If overwritten to call user code, include suitable
   // try/catch wrapping and send any errors to
-  // [_Zone.current.handleUncaughtError].
+  // [Zone._current.handleUncaughtError].
   void _onPause() {
     assert(_isInputPaused);
   }
@@ -352,7 +352,6 @@ class _BufferingStreamSubscription<T>
       // future to finish we must not report the error.
       if (_isCanceled && !_waitsForCancel) return;
       _state |= _STATE_IN_CALLBACK;
-      // TODO(floitsch): this dynamic should be 'void'.
       var onError = _onError;
       if (onError is void Function(Object, StackTrace)) {
         _zone.runBinaryGuarded<Object, StackTrace>(onError, error, stackTrace);
@@ -366,8 +365,7 @@ class _BufferingStreamSubscription<T>
       _state |= _STATE_WAIT_FOR_CANCEL;
       _cancel();
       var cancelFuture = _cancelFuture;
-      if (cancelFuture != null &&
-          !identical(cancelFuture, Future._nullFuture)) {
+      if (cancelFuture != null && !isNullFuture(Zone._current, cancelFuture)) {
         cancelFuture.whenComplete(sendError);
       } else {
         sendError();
@@ -396,7 +394,7 @@ class _BufferingStreamSubscription<T>
     _cancel();
     _state |= _STATE_WAIT_FOR_CANCEL;
     var cancelFuture = _cancelFuture;
-    if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
+    if (cancelFuture != null && !isNullFuture(Zone._current, cancelFuture)) {
       cancelFuture.whenComplete(sendDone);
     } else {
       sendDone();
@@ -672,7 +670,7 @@ class _DoneStreamSubscription<T> implements StreamSubscription<T> {
     }
   }
 
-  Future cancel() => Future._nullFuture;
+  Future cancel() => nullFuture;
 
   Future<E> asFuture<E>([E? futureValue]) {
     E resultValue;
@@ -819,7 +817,7 @@ class _BroadcastSubscriptionWrapper<T> implements StreamSubscription<T> {
 
   Future cancel() {
     _stream._cancelSubscription();
-    return Future._nullFuture;
+    return nullFuture;
   }
 
   bool get isPaused {
@@ -963,7 +961,7 @@ class _StreamIterator<T> implements StreamIterator<T> {
       }
       return subscription.cancel();
     }
-    return Future._nullFuture;
+    return nullFuture;
   }
 
   void _onData(T data) {

sdk/lib/async/stream_pipe.dart

@@ -26,7 +26,7 @@ _runUserCode<T>(T userCode(), onSuccess(T value),
 void _cancelAndError(StreamSubscription subscription, _Future future,
     Object error, StackTrace stackTrace) {
   var cancelFuture = subscription.cancel();
-  if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
+  if (cancelFuture != null && !isNullFuture(Zone._current, cancelFuture)) {
     cancelFuture.whenComplete(() => future._completeError(error, stackTrace));
   } else {
     future._completeError(error, stackTrace);
@@ -55,7 +55,7 @@ void Function(Object error, StackTrace stackTrace) _cancelAndErrorClosure(
   before completing with a value. */
 void _cancelAndValue(StreamSubscription subscription, _Future future, value) {
   var cancelFuture = subscription.cancel();
-  if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
+  if (cancelFuture != null && !isNullFuture(Zone._current, cancelFuture)) {
     cancelFuture.whenComplete(() => future._complete(value));
   } else {
     future._complete(value);

sdk/lib/internal/internal.dart

@@ -136,7 +136,7 @@ int parseHexByte(String source, int index) {
   return digit1 * 16 + digit2 - (digit2 & 256);
 }
 
-/// A reusable `null`-valued future used by `dart:async`.
+/// A reusable `null`-valued future per zone used by `dart:async`.
 ///
 /// **DO NOT USE.**
 ///
@@ -156,7 +156,14 @@ int parseHexByte(String source, int index) {
 /// This future will be removed again if we can ever do so.
 /// Do not use it for anything other than preserving timing
 /// during the null safety migration.
-final Future<Null> nullFuture = Zone.root.run(() => Future<Null>.value(null));
+Future<Null> get nullFuture =>
+    _nullFutures[Zone.current] ??= Future<Null>.value(null);
+
+/// Whether [future] is the null future of the current zone.
+bool isNullFuture(Zone zone, Future future) =>
+    identical(_nullFutures[zone], future);
+
+final Expando<Future<Null>> _nullFutures = Expando<Future<Null>>();
 
 /// A default hash function used by the platform in various places.
 ///

tests/lib/async/null_future_zone_test.dart

@@ -13,21 +13,33 @@ main() {
     Expect.isFalse(await it.moveNext());
 
     late Future nullFuture;
-    late Future falseFuture;
 
+    bool nullFutureZoneUsed = false;
     runZoned(() {
+      // Known code that exposes the special "nullFuture".
       nullFuture = (new StreamController()..stream.listen(null).cancel()).done;
-      falseFuture = it.moveNext();
     }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
         (Zone self, ZoneDelegate parent, Zone zone, void f()) {
-      Expect.fail("Should not be called");
+      Expect.identical(zone, self);
+      nullFutureZoneUsed = true;
+      parent.scheduleMicrotask(zone, f);
     }));
 
     nullFuture.then((value) {
       Expect.isNull(value);
+      Expect.isTrue(nullFutureZoneUsed);
       asyncEnd();
     });
 
+    late Future falseFuture;
+
+    runZoned(() {
+      falseFuture = it.moveNext();
+    }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
+        (Zone self, ZoneDelegate parent, Zone zone, void f()) {
+      Expect.fail("Should not be called");
+    }));
+
     falseFuture.then((value) {
       Expect.isFalse(value);
       asyncEnd();

tests/lib_2/async/null_future_zone_test.dart

@@ -15,21 +15,32 @@ main() {
     Expect.isFalse(await it.moveNext());
 
     Future nullFuture;
-    Future falseFuture;
 
+    bool nullFutureZoneUsed = false;
     runZoned(() {
       nullFuture = (new StreamController()..stream.listen(null).cancel()).done;
-      falseFuture = it.moveNext();
     }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
         (Zone self, ZoneDelegate parent, Zone zone, void f()) {
-      Expect.fail("Should not be called");
+      Expect.identical(zone, self);
+      nullFutureZoneUsed = true;
+      parent.scheduleMicrotask(zone, f);
     }));
 
     nullFuture.then((value) {
       Expect.isNull(value);
+      Expect.isTrue(nullFutureZoneUsed);
       asyncEnd();
     });
 
+    Future falseFuture;
+
+    runZoned(() {
+      falseFuture = it.moveNext();
+    }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
+        (Zone self, ZoneDelegate parent, Zone zone, void f()) {
+      Expect.fail("Should not be called");
+    }));
+
     falseFuture.then((value) {
       Expect.isFalse(value);
       asyncEnd();