Reland "Fix bug in Completer.complete."
This reverts commit 212d5e3a31e82f156ba1e9996ff2cf70d959582d.
This reapplies commit 3688a4723c3ea3c211003fbc899571d6ea4c8648.
CoreLibraryReviewExempt: Refactoring and relanding.
Change-Id: I9b52ef4073bfff50699919f8a5b85e5ff66d0c3b
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/258929
Reviewed-by: Nate Bosch <nbosch@google.com>
Commit-Queue: Nate Bosch <nbosch@google.com>
Auto-Submit: Lasse Nielsen <lrn@google.com>
diff --git a/sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart b/sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart
index 553de50..997e724 100644
--- a/sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart
+++ b/sdk/lib/_internal/js_dev_runtime/patch/async_patch.dart
@@ -98,7 +98,11 @@
// instead treat it as a completed value.
if (value is Future) {
if (value is _Future) {
- _Future._chainCoreFuture(value, asyncFuture);
+ if (isRunningAsEvent) {
+ _Future._chainCoreFutureSync(value, asyncFuture);
+ } else {
+ _Future._chainCoreFutureAsync(value, asyncFuture);
+ }
} else {
asyncFuture._chainForeignFuture(value);
}
@@ -108,7 +112,7 @@
asyncFuture._asyncComplete(JS('', '#', value));
}
} else {
- _Future._chainCoreFuture(onAwait(value), asyncFuture);
+ _Future._chainCoreFutureSync(onAwait(value), asyncFuture);
}
} catch (e, s) {
if (isRunningAsEvent) {
diff --git a/sdk/lib/async/future_impl.dart b/sdk/lib/async/future_impl.dart
index 812fff4..b92c5b0 100644
--- a/sdk/lib/async/future_impl.dart
+++ b/sdk/lib/async/future_impl.dart
@@ -7,6 +7,7 @@
abstract class _Completer<T> implements Completer<T> {
final _Future<T> future = new _Future<T>();
+ // Overridden by either a synchronous or asynchronous implementation.
void complete([FutureOr<T>? value]);
void completeError(Object error, [StackTrace? stackTrace]) {
@@ -23,6 +24,7 @@
_completeError(error, stackTrace);
}
+ // Overridden by either a synchronous or asynchronous implementation.
void _completeError(Object error, StackTrace stackTrace);
// The future's _isComplete doesn't take into account pending completions.
@@ -30,6 +32,7 @@
bool get isCompleted => !future._mayComplete;
}
+/// Completer which completes future asynchronously.
class _AsyncCompleter<T> extends _Completer<T> {
void complete([FutureOr<T>? value]) {
if (!future._mayComplete) throw new StateError("Future already completed");
@@ -41,6 +44,9 @@
}
}
+/// Completer which completes future synchronously.
+///
+/// Created by [Completer.sync]. Use with caution.
class _SyncCompleter<T> extends _Completer<T> {
void complete([FutureOr<T>? value]) {
if (!future._mayComplete) throw new StateError("Future already completed");
@@ -267,6 +273,7 @@
// This constructor is used by async/await.
_Future() : _zone = Zone._current;
+ // Constructor used by [Future.value].
_Future.immediate(FutureOr<T> result) : _zone = Zone._current {
_asyncComplete(result);
}
@@ -505,10 +512,13 @@
return prev;
}
- // Take the value (when completed) of source and complete this future with that
- // value (or error). This function could chain all Futures, but is slower
- // for _Future than _chainCoreFuture, so you must use _chainCoreFuture
- // in that case.
+ /// Completes this future with the result of [source].
+ ///
+ /// The [source] future should not be a [_Future], use
+ /// [_chainCoreFutureSync] for those.
+ ///
+ /// Since [source] is an unknown [Future], it's interacted with
+ /// through [Future.then], which is required to be asynchronous.
void _chainForeignFuture(Future source) {
assert(!_isComplete);
assert(source is! _Future);
@@ -539,9 +549,13 @@
}
}
- // Take the value (when completed) of source and complete target with that
- // value (or error). This function expects that source is a _Future.
- static void _chainCoreFuture(_Future source, _Future target) {
+ /// Synchronously completes a target future with another, source, future.
+ ///
+ /// If the source future is already completed, its result is synchronously
+ /// propagated to the target future's listeners.
+ /// If the source future is not completed, the target future is made
+ /// to listen for its completion.
+ static void _chainCoreFutureSync(_Future source, _Future target) {
assert(target._mayAddListener); // Not completed, not already chained.
while (source._isChained) {
source = source._chainSource;
@@ -557,11 +571,51 @@
}
}
+ /// Asynchronously completes a [target] future with a [source] future.
+ ///
+ /// If the [source] future is already completed, its result is
+ /// asynchronously propagated to the [target] future's listeners.
+ /// If the [source] future is not completed, the [target] future is made
+ /// to listen for its completion.
+ static void _chainCoreFutureAsync(_Future source, _Future target) {
+ assert(target._mayAddListener); // Not completed, not already chained.
+ while (source._isChained) {
+ source = source._chainSource;
+ }
+ if (!source._isComplete) {
+ // Chain immediately if the source is not complete.
+ // This won't call any listeners.
+ _FutureListener? listeners = target._resultOrListeners;
+ target._setChained(source);
+ source._prependListeners(listeners);
+ return;
+ }
+
+ // Complete a value synchronously, if no-one is listening.
+ // This won't call any listeners.
+ if (!source._hasError && target._resultOrListeners == null) {
+ target._cloneResult(source);
+ return;
+ }
+
+ // Otherwise delay the chaining to avoid any synchronous callbacks.
+ target._setPendingComplete();
+ target._zone.scheduleMicrotask(() {
+ _chainCoreFutureSync(source, target);
+ });
+ }
+
+ /// Synchronously completes this future with [value].
+ ///
+ /// If [value] is a value or an already completed [_Future],
+ /// the result is immediately used to complete this future.
+ /// If [value] is an incomplete future, this future will wait for
+ /// it to complete, then use the result.
void _complete(FutureOr<T> value) {
assert(!_isComplete);
if (value is Future<T>) {
if (value is _Future<T>) {
- _chainCoreFuture(value, this);
+ _chainCoreFutureSync(value, this);
} else {
_chainForeignFuture(value);
}
@@ -592,8 +646,9 @@
_propagateToListeners(this, listeners);
}
+ // Completes future in a later microtask.
void _asyncComplete(FutureOr<T> value) {
- assert(!_isComplete);
+ assert(!_isComplete); // Allows both pending complete and incomplete.
// Two corner cases if the value is a future:
// 1. the future is already completed and an error.
// 2. the future is not yet completed but might become an error.
@@ -658,17 +713,15 @@
});
}
+ /// Asynchronously completes a future with another future.
+ ///
+ /// Even if [value] is already completed, it won't synchronously
+ /// complete this completer's future.
void _chainFuture(Future<T> value) {
+ assert(_mayComplete);
if (value is _Future<T>) {
- if (value._hasError) {
- // Delay completion to allow the user to register callbacks.
- _setPendingComplete();
- _zone.scheduleMicrotask(() {
- _chainCoreFuture(value, this);
- });
- } else {
- _chainCoreFuture(value, this);
- }
+ // Chain ensuring that we don't complete synchronously.
+ _chainCoreFutureAsync(value, this);
return;
}
// Just listen on the foreign future. This guarantees an async delay.
@@ -836,7 +889,7 @@
source = chainSource;
continue;
} else {
- _chainCoreFuture(chainSource, result);
+ _chainCoreFutureSync(chainSource, result);
}
} else {
result._chainForeignFuture(chainSource);
diff --git a/tests/lib/async/future_regression_48493_test.dart b/tests/lib/async/future_regression_48493_test.dart
new file mode 100644
index 0000000..b452da5
--- /dev/null
+++ b/tests/lib/async/future_regression_48493_test.dart
@@ -0,0 +1,31 @@
+// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Regression test for http://dartbug.com/48493
+//
+// The `Completer.complete` method, given an already completed `_Future`
+// with a value result, will complete the `Completer.future` synchronously.
+
+import 'package:async_helper/async_helper.dart';
+import "package:expect/expect.dart";
+import 'dart:async';
+
+void main() async {
+ asyncStart();
+
+ var completer = Completer<int>(); // Not synchronous.
+ var future = Future<int>.value(4);
+ await future; // Ensure future completed.
+ bool thenRun = false;
+ completer.future.then((_) {
+ thenRun = true;
+ });
+ Expect.isFalse(thenRun, "Sanity check");
+ completer.complete(future);
+ Expect.isFalse(thenRun, "Ran early"); // Bug would cause this to fail.
+ await Future(() {}); // Await timer, all microtasks completed.
+ Expect.isTrue(thenRun, "Did not run as microtask");
+
+ asyncEnd();
+}
diff --git a/tests/lib_2/async/future_regression_48493_test.dart b/tests/lib_2/async/future_regression_48493_test.dart
new file mode 100644
index 0000000..e167bf3
--- /dev/null
+++ b/tests/lib_2/async/future_regression_48493_test.dart
@@ -0,0 +1,33 @@
+// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// @dart = 2.9
+
+// Regression test for http://dartbug.com/48493
+//
+// The `Completer.complete` method, given an already completed `_Future`
+// with a value result, will complete the `Completer.future` synchronously.
+
+import 'package:async_helper/async_helper.dart';
+import "package:expect/expect.dart";
+import 'dart:async';
+
+void main() async {
+ asyncStart();
+
+ var completer = Completer<int>(); // Not synchronous.
+ var future = Future<int>.value(4);
+ await future; // Ensure future completed.
+ bool thenRun = false;
+ completer.future.then((_) {
+ thenRun = true;
+ });
+ Expect.isFalse(thenRun, "Sanity check");
+ completer.complete(future);
+ Expect.isFalse(thenRun, "Ran early"); // Bug would cause this to fail.
+ await Future(() {}); // Await timer, all microtasks completed.
+ Expect.isTrue(thenRun, "Did not run as microtask");
+
+ asyncEnd();
+}
