Revert "Make `nullFuture` be per-zone."
This reverts commit 4d750a862d3f659107e0101d3035bcdc2ca3b2ee.
Reason for revert: breaks google3 (b/236665701)
Original change's description:
> 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 a247b158d6fc2ae89e824080bd7877e7b51f642a
>
> Change-Id: Ia113756de1f6d50af4b1abfec219d6b4dcd5d59b
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249488
> Reviewed-by: Nate Bosch <nbosch@google.com>
> Commit-Queue: Lasse Nielsen <lrn@google.com>
TBR=lrn@google.com,nbosch@google.com
Change-Id: I02d62d58bae33d6a606a80eb3eee2e8e721a8e20
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249620
Commit-Queue: Emmanuel Pellereau <emmanuelp@google.com>
Reviewed-by: Emmanuel Pellereau <emmanuelp@google.com>
Reviewed-by: Nate Bosch <nbosch@google.com>
diff --git a/pkg/front_end/testcases/nnbd/issue42546.dart.weak.outline.expect b/pkg/front_end/testcases/nnbd/issue42546.dart.weak.outline.expect
index ec6a7f6..e218902 100644
--- a/pkg/front_end/testcases/nnbd/issue42546.dart.weak.outline.expect
+++ b/pkg/front_end/testcases/nnbd/issue42546.dart.weak.outline.expect
@@ -28,19 +28,19 @@
Extra constant evaluation status:
-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>{})
+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>{})
Extra constant evaluation: evaluated: 61, effectively constant: 15
diff --git a/sdk/lib/async/async.dart b/sdk/lib/async/async.dart
index bd41381..ca05fc6 100644
--- a/sdk/lib/async/async.dart
+++ b/sdk/lib/async/async.dart
@@ -110,6 +110,7 @@
checkNotNullable,
EmptyIterator,
IterableElementError,
+ nullFuture,
printToZone,
printToConsole,
Since,
diff --git a/sdk/lib/async/future.dart b/sdk/lib/async/future.dart
index 908e5e1..f83386d 100644
--- a/sdk/lib/async/future.dart
+++ b/sdk/lib/async/future.dart
@@ -223,6 +223,13 @@
/// 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);
diff --git a/sdk/lib/async/stream.dart b/sdk/lib/async/stream.dart
index 117465c..038c3bb 100644
--- a/sdk/lib/async/stream.dart
+++ b/sdk/lib/async/stream.dart
@@ -515,7 +515,7 @@
controller
..onCancel = () {
timer.cancel();
- return Zone._current._nullFuture;
+ return Future._nullFuture;
}
..onPause = () {
watch.stop();
diff --git a/sdk/lib/async/stream_controller.dart b/sdk/lib/async/stream_controller.dart
index 1f34d76..2baf008 100644
--- a/sdk/lib/async/stream_controller.dart
+++ b/sdk/lib/async/stream_controller.dart
@@ -588,10 +588,7 @@
Future<void> get done => _ensureDoneFuture();
Future<void> _ensureDoneFuture() =>
- _doneFuture ??
- (_isCanceled
- ? Zone._current._nullFuture as Future<void>
- : _doneFuture = _Future<void>());
+ _doneFuture ??= _isCanceled ? Future._nullFuture : _Future<void>();
/// Send or enqueue a data event.
void add(T value) {
@@ -922,7 +919,7 @@
var cancel = addSubscription.cancel();
if (cancel == null) {
addStreamFuture._asyncComplete(null);
- return Zone._current._nullFuture;
+ return Future._nullFuture;
}
return cancel.whenComplete(() {
addStreamFuture._asyncComplete(null);
diff --git a/sdk/lib/async/stream_impl.dart b/sdk/lib/async/stream_impl.dart
index b5a40cb..b4fb5d8 100644
--- a/sdk/lib/async/stream_impl.dart
+++ b/sdk/lib/async/stream_impl.dart
@@ -197,7 +197,7 @@
if (!_isCanceled) {
_cancel();
}
- return _cancelFuture ?? Zone._current._nullFuture;
+ return _cancelFuture ?? Future._nullFuture;
}
Future<E> asFuture<E>([E? futureValue]) {
@@ -217,7 +217,7 @@
};
_onError = (Object error, StackTrace stackTrace) {
Future cancelFuture = cancel();
- if (!identical(Zone._current._nullFuture, cancelFuture)) {
+ if (!identical(cancelFuture, Future._nullFuture)) {
cancelFuture.whenComplete(() {
result._completeError(error, stackTrace);
});
@@ -297,7 +297,7 @@
// 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,6 +352,7 @@
// 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,7 +367,7 @@
_cancel();
var cancelFuture = _cancelFuture;
if (cancelFuture != null &&
- !identical(Zone._current._nullFuture, cancelFuture)) {
+ !identical(cancelFuture, Future._nullFuture)) {
cancelFuture.whenComplete(sendError);
} else {
sendError();
@@ -395,8 +396,7 @@
_cancel();
_state |= _STATE_WAIT_FOR_CANCEL;
var cancelFuture = _cancelFuture;
- if (cancelFuture != null &&
- !identical(Zone._current._nullFuture, cancelFuture)) {
+ if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
cancelFuture.whenComplete(sendDone);
} else {
sendDone();
@@ -672,7 +672,7 @@
}
}
- Future cancel() => Zone._current._nullFuture;
+ Future cancel() => Future._nullFuture;
Future<E> asFuture<E>([E? futureValue]) {
E resultValue;
@@ -819,7 +819,7 @@
Future cancel() {
_stream._cancelSubscription();
- return Zone._current._nullFuture;
+ return Future._nullFuture;
}
bool get isPaused {
@@ -963,7 +963,7 @@
}
return subscription.cancel();
}
- return Zone._current._nullFuture;
+ return Future._nullFuture;
}
void _onData(T data) {
diff --git a/sdk/lib/async/stream_pipe.dart b/sdk/lib/async/stream_pipe.dart
index 2caa84b..cd707be 100644
--- a/sdk/lib/async/stream_pipe.dart
+++ b/sdk/lib/async/stream_pipe.dart
@@ -26,8 +26,7 @@
void _cancelAndError(StreamSubscription subscription, _Future future,
Object error, StackTrace stackTrace) {
var cancelFuture = subscription.cancel();
- if (cancelFuture != null &&
- !identical(Zone._current._nullFuture, cancelFuture)) {
+ if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
cancelFuture.whenComplete(() => future._completeError(error, stackTrace));
} else {
future._completeError(error, stackTrace);
@@ -56,8 +55,7 @@
before completing with a value. */
void _cancelAndValue(StreamSubscription subscription, _Future future, value) {
var cancelFuture = subscription.cancel();
- if (cancelFuture != null &&
- !identical(Zone._current._nullFuture, cancelFuture)) {
+ if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
cancelFuture.whenComplete(() => future._complete(value));
} else {
future._complete(value);
diff --git a/sdk/lib/async/zone.dart b/sdk/lib/async/zone.dart
index 42e0c76..b9bcd19 100644
--- a/sdk/lib/async/zone.dart
+++ b/sdk/lib/async/zone.dart
@@ -1078,28 +1078,6 @@
implZone, e, identical(error, e) ? stackTrace : s);
}
}
-
- /// A reusable `null`-valued future per zone used by `dart:async`.
- ///
- /// **DO NOT USE.**
- ///
- /// This future is used in situations where a future is expected,
- /// but no asynchronous computation actually happens,
- /// like cancelling a stream from a controller with no `onCancel` callback.
- /// *Some code depends on recognizing this future in order to react
- /// synchronously.*
- /// It does so to avoid changing event interleaving during the null safety
- /// migration where, for example, the [StreamSubscription.cancel] method
- /// stopped being able to return `null`.
- /// The code that would be broken by such a timing change is fragile,
- /// but we are not able to simply change it.
- /// For better or worse, code depends on the precise timing
- /// that our libraries have so far exhibited.
- ///
- /// 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.
- _Future<Null> get _nullFuture;
}
class _CustomZone extends _Zone {
@@ -1127,9 +1105,6 @@
/// The parent zone.
final _Zone parent;
- /// Cached value for [_nullFuture];
- _Future<Null>? _nullFutureCache;
-
/// The zone's scoped value declaration map.
///
/// This is always a [HashMap].
@@ -1221,18 +1196,6 @@
/// parent's error-zone.
Zone get errorZone => _handleUncaughtError.zone;
- _Future<Null> get _nullFuture {
- _Future<Null>? result = _nullFutureCache;
- if (result != null) return result;
- // We only care about the zone of the null future
- // because of the zone it schedules microtasks in.
- var microtaskZone = _scheduleMicrotask.zone;
- if (!identical(microtaskZone, this)) {
- return _nullFutureCache = microtaskZone._nullFuture;
- }
- return _nullFutureCache = _Future<Null>.value(null);
- }
-
void runGuarded(void f()) {
try {
run(f);
@@ -1546,8 +1509,6 @@
}
class _RootZone extends _Zone {
- static final _nullFutureCache = _Future<Null>.zoneValue(null, _rootZone);
-
const _RootZone();
_ZoneFunction<RunHandler> get _run =>
@@ -1586,8 +1547,6 @@
// The parent zone.
_Zone? get parent => null;
- _Future<Null> get _nullFuture => _nullFutureCache;
-
/// The zone's scoped value declaration map.
///
/// This is always a [HashMap].
diff --git a/sdk/lib/html/dart2js/html_dart2js.dart b/sdk/lib/html/dart2js/html_dart2js.dart
index 91224f6..671f68a 100644
--- a/sdk/lib/html/dart2js/html_dart2js.dart
+++ b/sdk/lib/html/dart2js/html_dart2js.dart
@@ -37302,13 +37302,13 @@
}
Future cancel() {
- if (!_canceled) {
- _unlisten();
- // Clear out the target to indicate this is complete.
- _target = null;
- _onData = null;
- }
- return Future<void>.value(null);
+ if (_canceled) return nullFuture;
+
+ _unlisten();
+ // Clear out the target to indicate this is complete.
+ _target = null;
+ _onData = null;
+ return nullFuture;
}
bool get _canceled => _target == null;
diff --git a/sdk/lib/internal/internal.dart b/sdk/lib/internal/internal.dart
index 0353c71..ff3d890 100644
--- a/sdk/lib/internal/internal.dart
+++ b/sdk/lib/internal/internal.dart
@@ -136,7 +136,27 @@
return digit1 * 16 + digit2 - (digit2 & 256);
}
-final Expando<Future<Null>> _nullFutures = Expando<Future<Null>>();
+/// A reusable `null`-valued future used by `dart:async`.
+///
+/// **DO NOT USE.**
+///
+/// This future is used in situations where a future is expected,
+/// but no asynchronous computation actually happens,
+/// like cancelling a stream from a controller with no `onCancel` callback.
+/// *Some code depends on recognizing this future in order to react
+/// synchronously.*
+/// It does so to avoid changing event interleaving during the null safety
+/// migration where, for example, the [StreamSubscription.cancel] method
+/// stopped being able to return `null`.
+/// The code that would be broken by such a timing change is fragile,
+/// but we are not able to simply change it.
+/// For better or worse, code depends on the precise timing that our libraries
+/// have so far exhibited.
+///
+/// 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));
/// A default hash function used by the platform in various places.
///
diff --git a/tests/lib/async/null_future_zone_test.dart b/tests/lib/async/null_future_zone_test.dart
index 34201e7..9a266be 100644
--- a/tests/lib/async/null_future_zone_test.dart
+++ b/tests/lib/async/null_future_zone_test.dart
@@ -13,32 +13,21 @@
Expect.isFalse(await it.moveNext());
late Future nullFuture;
-
- bool nullFutureZoneUsed = false;
- runZoned(() {
- nullFuture = (new StreamController()..stream.listen(null).cancel()).done;
- }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
- (Zone self, ZoneDelegate parent, Zone zone, void f()) {
- Expect.identical(zone, self);
- nullFutureZoneUsed = true;
- parent.scheduleMicrotask(zone, f);
- }));
-
- nullFuture.then((value) {
- Expect.isNull(value);
- Expect.isTrue(nullFutureZoneUsed);
- asyncEnd();
- });
-
late Future falseFuture;
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");
}));
+ nullFuture.then((value) {
+ Expect.isNull(value);
+ asyncEnd();
+ });
+
falseFuture.then((value) {
Expect.isFalse(value);
asyncEnd();
diff --git a/tests/lib_2/async/null_future_zone_test.dart b/tests/lib_2/async/null_future_zone_test.dart
index 77cb1b7..3322916 100644
--- a/tests/lib_2/async/null_future_zone_test.dart
+++ b/tests/lib_2/async/null_future_zone_test.dart
@@ -15,32 +15,21 @@
Expect.isFalse(await it.moveNext());
Future nullFuture;
-
- bool nullFutureZoneUsed = false;
- runZoned(() {
- nullFuture = (new StreamController()..stream.listen(null).cancel()).done;
- }, zoneSpecification: new ZoneSpecification(scheduleMicrotask:
- (Zone self, ZoneDelegate parent, Zone zone, void f()) {
- Expect.identical(zone, self);
- nullFutureZoneUsed = true;
- parent.scheduleMicrotask(zone, f);
- }));
-
- nullFuture.then((value) {
- Expect.isNull(value);
- Expect.isTrue(nullFutureZoneUsed);
- asyncEnd();
- });
-
Future falseFuture;
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");
}));
+ nullFuture.then((value) {
+ Expect.isNull(value);
+ asyncEnd();
+ });
+
falseFuture.then((value) {
Expect.isFalse(value);
asyncEnd();
diff --git a/tools/dom/src/EventStreamProvider.dart b/tools/dom/src/EventStreamProvider.dart
index 1ae6268..90b19aa 100644
--- a/tools/dom/src/EventStreamProvider.dart
+++ b/tools/dom/src/EventStreamProvider.dart
@@ -248,13 +248,13 @@
}
Future cancel() {
- if (!_canceled) {
- _unlisten();
- // Clear out the target to indicate this is complete.
- _target = null;
- _onData = null;
- }
- return Future<void>.value(null);
+ if (_canceled) return nullFuture;
+
+ _unlisten();
+ // Clear out the target to indicate this is complete.
+ _target = null;
+ _onData = null;
+ return nullFuture;
}
bool get _canceled => _target == null;
