Don't allow completing a _Future with itself.

This never worked. It was silently accepted, at least if the future had no listeners, then any later attempt to use the future would cause a stack overflow or other impossible results. Moves some `_Future._complete` call out of try-catch. The `_complete` shouldn't throw (but before this fix it could). Moving them out of the `try`/`catch` makes such errors be reported as unhandled, instead of catching them and trying to complete the same future again with an error, when it's possibly in an inconsistent state. Fixes #43662. Based on #43662 (comment) CoreLibraryReviewExempt: No response. Bug: http://dartbug.com/43662 Change-Id: I96a4f01bcd5b6cee93bba267299852569a9b905c Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/363060 Commit-Queue: Lasse Nielsen <lrn@google.com> Reviewed-by: Nate Bosch <nbosch@google.com> Reviewed-by: Martin Kustermann <kustermann@google.com>
dart-lang · May 14, 2024 · b06a34f · b06a34f
1 parent e4c1985
commit b06a34f
Show file tree

Hide file tree

Showing 51 changed files with 237 additions and 103 deletions.
diff --git a/sdk/lib/async/future.dart b/sdk/lib/async/future.dart
@@ -253,11 +253,14 @@ abstract interface class Future<T> {
   factory Future(FutureOr<T> computation()) {
     _Future<T> result = new _Future<T>();
     Timer.run(() {
+      FutureOr<T> computationResult;
       try {
-        result._complete(computation());
+        computationResult = computation();
       } catch (e, s) {
         _completeWithErrorCallback(result, e, s);
+        return;
       }
+      result._complete(computationResult);
     });
     return result;
   }
@@ -277,11 +280,14 @@ abstract interface class Future<T> {
   factory Future.microtask(FutureOr<T> computation()) {
     _Future<T> result = new _Future<T>();
     scheduleMicrotask(() {
+      FutureOr<T> computationResult;
       try {
-        result._complete(computation());
+        computationResult = computation();
       } catch (e, s) {
         _completeWithErrorCallback(result, e, s);
+        return;
       }
+      result._complete(computationResult);
     });
     return result;
   }
@@ -302,9 +308,9 @@ abstract interface class Future<T> {
   /// final result = await Future<int>.sync(() => 12);
   /// ```
   factory Future.sync(FutureOr<T> computation()) {
+    FutureOr<T> result;
     try {
-      var result = computation();
-      return result is Future<T> ? result : _Future<T>.value(result);
+      result = computation();
     } catch (error, stackTrace) {
       var future = new _Future<T>();
       AsyncError? replacement = Zone.current.errorCallback(error, stackTrace);
@@ -315,6 +321,7 @@ abstract interface class Future<T> {
       }
       return future;
     }
+    return result is Future<T> ? result : _Future<T>.value(result);
   }
 
   /// Creates a future completed with [value].
@@ -414,16 +421,19 @@ abstract interface class Future<T> {
       throw ArgumentError.value(
           null, "computation", "The type parameter is not nullable");
     }
-    _Future<T> result = new _Future<T>();
+    _Future<T> result = _Future<T>();
     new Timer(duration, () {
       if (computation == null) {
         result._complete(null as T);
       } else {
+        FutureOr<T> computationResult;
         try {
-          result._complete(computation());
+          computationResult = computation();
         } catch (e, s) {
           _completeWithErrorCallback(result, e, s);
+          return;
         }
+        result._complete(computationResult);
       }
     });
     return result;

diff --git a/sdk/lib/async/future_impl.dart b/sdk/lib/async/future_impl.dart
@@ -402,7 +402,7 @@ class _Future<T> implements Future<T> {
   Stream<T> asStream() => new Stream<T>.fromFuture(this);
 
   void _setPendingComplete() {
-    assert(_mayComplete); // Aka _statIncomplete
+    assert(_mayComplete); // Aka. _stateIncomplete
     _state ^= _stateIncomplete ^ _statePendingComplete;
   }
 
@@ -577,6 +577,13 @@ class _Future<T> implements Future<T> {
     while (source._isChained) {
       source = source._chainSource;
     }
+    if (identical(source, target)) {
+      target._asyncCompleteError(
+          ArgumentError.value(
+              source, null, "Cannot complete a future with itself"),
+          StackTrace.current);
+      return;
+    }
     source._state |= target._state & _stateIgnoreError;
     if (source._isComplete) {
       _FutureListener? listeners = target._removeListeners();
@@ -600,6 +607,13 @@ class _Future<T> implements Future<T> {
     while (source._isChained) {
       source = source._chainSource;
     }
+    if (identical(source, target)) {
+      target._asyncCompleteError(
+          ArgumentError.value(
+              source, null, "Cannot complete a future with itself"),
+          StackTrace.current);
+      return;
+    }
     if (!source._isComplete) {
       // Chain immediately if the source is not complete.
       // This won't call any listeners.

diff --git a/tests/lib/async/catch_errors11_test.dart b/tests/lib/async/catch_errors11_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors12_test.dart b/tests/lib/async/catch_errors12_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors13_test.dart b/tests/lib/async/catch_errors13_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors14_test.dart b/tests/lib/async/catch_errors14_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors15_test.dart b/tests/lib/async/catch_errors15_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors16_test.dart b/tests/lib/async/catch_errors16_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors17_test.dart b/tests/lib/async/catch_errors17_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors18_test.dart b/tests/lib/async/catch_errors18_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors19_test.dart b/tests/lib/async/catch_errors19_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors20_test.dart b/tests/lib/async/catch_errors20_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors21_test.dart b/tests/lib/async/catch_errors21_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors22_test.dart b/tests/lib/async/catch_errors22_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors23_test.dart b/tests/lib/async/catch_errors23_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors24_test.dart b/tests/lib/async/catch_errors24_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors25_test.dart b/tests/lib/async/catch_errors25_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors26_test.dart b/tests/lib/async/catch_errors26_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors27_test.dart b/tests/lib/async/catch_errors27_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors28_test.dart b/tests/lib/async/catch_errors28_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors2_test.dart b/tests/lib/async/catch_errors2_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {

diff --git a/tests/lib/async/catch_errors3_test.dart b/tests/lib/async/catch_errors3_test.dart
@@ -2,9 +2,11 @@
 // 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.
 
-import 'package:async_helper/async_helper.dart';
-import "package:expect/expect.dart";
 import 'dart:async';
+
+import 'package:async_helper/async_helper.dart';
+import 'package:expect/expect.dart';
+
 import 'catch_errors.dart';
 
 main() {