Revert "Make nullFuture be per-zone."

This reverts commit c97f7b7fad.

Reason for revert: Still breaking 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 a247b158d6
>
> Change-Id: Ieec74d6f93c57175c357ec18889144635f5bdca6
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249490
> Commit-Queue: Lasse Nielsen <lrn@google.com>
> Reviewed-by: Erik Ernst <eernst@google.com>

TBR=lrn@google.com,eernst@google.com,nbosch@google.com

Change-Id: I870285b03ec05803c5aaa6b66f9a6e6ea360d732
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/249609
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Emmanuel Pellereau <emmanuelp@google.com>
Reviewed-by: Emmanuel Pellereau <emmanuelp@google.com>
This commit is contained in:
Emmanuel Pellereau
2022-06-24 14:13:39 +00:00
committed by Commit Bot
parent e18977ed9c
commit fd590c54c2
10 changed files with 51 additions and 76 deletions
@@ -28,19 +28,19 @@ static method main() → dynamic
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
-1
View File
@@ -109,7 +109,6 @@ import "dart:_internal"
CastStreamTransformer,
checkNotNullable,
EmptyIterator,
isNullFuture,
IterableElementError,
nullFuture,
printToZone,
+7
View File
@@ -223,6 +223,13 @@ 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);
+1 -1
View File
@@ -515,7 +515,7 @@ abstract class Stream<T> {
controller
..onCancel = () {
timer.cancel();
return nullFuture;
return Future._nullFuture;
}
..onPause = () {
watch.stop();
+2 -5
View File
@@ -588,10 +588,7 @@ abstract class _StreamController<T> implements _StreamControllerBase<T> {
Future<void> get done => _ensureDoneFuture();
Future<void> _ensureDoneFuture() =>
_doneFuture ??
(_isCanceled
? 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 @@ class _AddStreamState<T> {
var cancel = addSubscription.cancel();
if (cancel == null) {
addStreamFuture._asyncComplete(null);
return nullFuture;
return Future._nullFuture;
}
return cancel.whenComplete(() {
addStreamFuture._asyncComplete(null);
+10 -8
View File
@@ -197,7 +197,7 @@ class _BufferingStreamSubscription<T>
if (!_isCanceled) {
_cancel();
}
return _cancelFuture ?? nullFuture;
return _cancelFuture ?? Future._nullFuture;
}
Future<E> asFuture<E>([E? futureValue]) {
@@ -217,7 +217,7 @@ class _BufferingStreamSubscription<T>
};
_onError = (Object error, StackTrace stackTrace) {
Future cancelFuture = cancel();
if (!isNullFuture(Zone._current, cancelFuture)) {
if (!identical(cancelFuture, Future._nullFuture)) {
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,6 +352,7 @@ 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);
@@ -365,7 +366,8 @@ class _BufferingStreamSubscription<T>
_state |= _STATE_WAIT_FOR_CANCEL;
_cancel();
var cancelFuture = _cancelFuture;
if (cancelFuture != null && !isNullFuture(Zone._current, cancelFuture)) {
if (cancelFuture != null &&
!identical(cancelFuture, Future._nullFuture)) {
cancelFuture.whenComplete(sendError);
} else {
sendError();
@@ -394,7 +396,7 @@ class _BufferingStreamSubscription<T>
_cancel();
_state |= _STATE_WAIT_FOR_CANCEL;
var cancelFuture = _cancelFuture;
if (cancelFuture != null && !isNullFuture(Zone._current, cancelFuture)) {
if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
cancelFuture.whenComplete(sendDone);
} else {
sendDone();
@@ -670,7 +672,7 @@ class _DoneStreamSubscription<T> implements StreamSubscription<T> {
}
}
Future cancel() => nullFuture;
Future cancel() => Future._nullFuture;
Future<E> asFuture<E>([E? futureValue]) {
E resultValue;
@@ -817,7 +819,7 @@ class _BroadcastSubscriptionWrapper<T> implements StreamSubscription<T> {
Future cancel() {
_stream._cancelSubscription();
return nullFuture;
return Future._nullFuture;
}
bool get isPaused {
@@ -961,7 +963,7 @@ class _StreamIterator<T> implements StreamIterator<T> {
}
return subscription.cancel();
}
return nullFuture;
return Future._nullFuture;
}
void _onData(T data) {
+2 -2
View File
@@ -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 && !isNullFuture(Zone._current, cancelFuture)) {
if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
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 && !isNullFuture(Zone._current, cancelFuture)) {
if (cancelFuture != null && !identical(cancelFuture, Future._nullFuture)) {
cancelFuture.whenComplete(() => future._complete(value));
} else {
future._complete(value);
+2 -9
View File
@@ -136,7 +136,7 @@ int parseHexByte(String source, int index) {
return digit1 * 16 + digit2 - (digit2 & 256);
}
/// A reusable `null`-valued future per zone used by `dart:async`.
/// A reusable `null`-valued future used by `dart:async`.
///
/// **DO NOT USE.**
///
@@ -156,14 +156,7 @@ 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.
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>>();
final Future<Null> nullFuture = Zone.root.run(() => Future<Null>.value(null));
/// A default hash function used by the platform in various places.
///
+6 -18
View File
@@ -13,33 +13,21 @@ main() {
Expect.isFalse(await it.moveNext());
late Future nullFuture;
bool nullFutureZoneUsed = false;
runZoned(() {
// Known code that exposes the special "nullFuture".
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();
+6 -17
View File
@@ -15,32 +15,21 @@ main() {
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();