From f0a8b634022c95b3b99161828d4f50a4fce7a9c5 Mon Sep 17 00:00:00 2001 From: George Wright Date: Thu, 22 Apr 2021 22:20:16 +0000 Subject: [PATCH] Revert "Make throwing Zone.uncaughtError handlers propagate the error to their parent zone." This reverts commit 88a351f3d2e675e203ce6452bc39f0e06e2a037c. This broke the Dart SDK -> Flutter Engine roller. Flutter issue at https://github.com/flutter/flutter/issues/80969 Change-Id: Idaf255a730c7b6054e6cd929b6770dbe66860151 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/196561 Reviewed-by: Zach Anderson Commit-Queue: Zach Anderson --- CHANGELOG.md | 6 -- sdk/lib/async/zone.dart | 92 ++----------------- .../uncaught_error_handler_throws_test.dart | 76 --------------- tests/lib_2/async/slow_consumer2_test.dart | 2 +- .../uncaught_error_handler_throws_test.dart | 76 --------------- 5 files changed, 10 insertions(+), 242 deletions(-) delete mode 100644 tests/lib/async/uncaught_error_handler_throws_test.dart delete mode 100644 tests/lib_2/async/uncaught_error_handler_throws_test.dart diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bf5a3a5e08..a45fa7a665e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,12 +2,6 @@ ### Core libraries -#### `dart:async` - -* The uncaught error handlers of `Zone`s are now run in the parent zone - of the zone where they were declared. This prevents a throwing handler - from causing an infinite loop by repeatedly triggering itself. - #### `dart:core` * The native `DateTime` class now better handles local time around diff --git a/sdk/lib/async/zone.dart b/sdk/lib/async/zone.dart index fe6dcf00c39..9301ce3086a 100644 --- a/sdk/lib/async/zone.dart +++ b/sdk/lib/async/zone.dart @@ -17,15 +17,6 @@ typedef R ZoneBinaryCallback(T1 arg1, T2 arg2); /// /// The [error] and [stackTrace] are the error and stack trace that /// was uncaught in [zone]. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). -/// -/// If the uncaught error handler throws, the error will be passed -/// to `parent.handleUncaughtError`. If the thrown object is [error], -/// the throw is considered a re-throw and the original [stackTrace] -/// is retained. This allows an asynchronous error to leave the error zone. typedef HandleUncaughtErrorHandler = void Function(Zone self, ZoneDelegate parent, Zone zone, Object error, StackTrace stackTrace); @@ -43,10 +34,6 @@ typedef HandleUncaughtErrorHandler = void Function(Zone self, /// to call [f] in the current zone, [zone]. /// A custom handler can do things before, after or instead of /// calling [f]. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef RunHandler = R Function( Zone self, ZoneDelegate parent, Zone zone, R Function() f); @@ -64,10 +51,6 @@ typedef RunHandler = R Function( /// to call [f] with argument [arg] in the current zone, [zone]. /// A custom handler can do things before, after or instead of /// calling [f]. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef RunUnaryHandler = R Function( Zone self, ZoneDelegate parent, Zone zone, R Function(T arg) f, T arg); @@ -85,10 +68,6 @@ typedef RunUnaryHandler = R Function( /// to call [f] with arguments [arg1] and [arg2] in the current zone, [zone]. /// A custom handler can do things before, after or instead of /// calling [f]. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef RunBinaryHandler = R Function(Zone self, ZoneDelegate parent, Zone zone, R Function(T1 arg1, T2 arg2) f, T1 arg1, T2 arg2); @@ -106,10 +85,6 @@ typedef RunBinaryHandler = R Function(Zone self, ZoneDelegate parent, /// or another function replacing [f], /// typically by wrapping [f] in a function /// which does something extra before and after invoking [f] -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef RegisterCallbackHandler = ZoneCallback Function( Zone self, ZoneDelegate parent, Zone zone, R Function() f); @@ -127,10 +102,6 @@ typedef RegisterCallbackHandler = ZoneCallback Function( /// or another function replacing [f], /// typically by wrapping [f] in a function /// which does something extra before and after invoking [f] -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef RegisterUnaryCallbackHandler = ZoneUnaryCallback Function( Zone self, ZoneDelegate parent, Zone zone, R Function(T arg) f); @@ -166,12 +137,6 @@ typedef RegisterBinaryCallbackHandler /// to replace the original error and stack trace, /// or an [AsyncError] containing a replacement error and stack trace /// which will be used to replace the originals. -/// -/// The error callback handler must not throw. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef AsyncError? ErrorCallbackHandler(Zone self, ZoneDelegate parent, Zone zone, Object error, StackTrace? stackTrace); @@ -190,10 +155,6 @@ typedef AsyncError? ErrorCallbackHandler(Zone self, ZoneDelegate parent, /// and then call `parent.scheduleMicrotask(zone, replacement)`. /// or it can implement its own microtask scheduling queue, which typically /// still depends on `parent.scheduleMicrotask` to as a way to get started. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef void ScheduleMicrotaskHandler( Zone self, ZoneDelegate parent, Zone zone, void f()); @@ -216,10 +177,6 @@ typedef void ScheduleMicrotaskHandler( /// /// The function should return a [Timer] object which can be used /// to inspect and control the scheduled timer callback. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef Timer CreateTimerHandler( Zone self, ZoneDelegate parent, Zone zone, Duration duration, void f()); @@ -242,10 +199,6 @@ typedef Timer CreateTimerHandler( /// /// The function should return a [Timer] object which can be used /// to inspect and control the scheduled timer callbacks. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef Timer CreatePeriodicTimerHandler(Zone self, ZoneDelegate parent, Zone zone, Duration period, void f(Timer timer)); @@ -261,10 +214,6 @@ typedef Timer CreatePeriodicTimerHandler(Zone self, ZoneDelegate parent, /// /// The custom handler can intercept print operations and /// redirect them to other targets than the console. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef void PrintHandler( Zone self, ZoneDelegate parent, Zone zone, String line); @@ -286,10 +235,6 @@ typedef void PrintHandler( /// values before calling `parent.fork(zone, specification, zoneValues)`, /// but it has to call the [parent]'s [ZoneDelegate.fork] in order /// to create a valid [Zone] object. -/// -/// The function must only access zone-related functionality through -/// [self], [parent] or [zone]. -/// It should not depend on the current zone ([Zone.current]). typedef Zone ForkHandler(Zone self, ZoneDelegate parent, Zone zone, ZoneSpecification? specification, Map? zoneValues); @@ -970,7 +915,10 @@ class _ZoneDelegate implements ZoneDelegate { _ZoneDelegate(this._delegationTarget); void handleUncaughtError(Zone zone, Object error, StackTrace stackTrace) { - _delegationTarget._processUncaughtError(zone, error, stackTrace); + var implementation = _delegationTarget._handleUncaughtError; + _Zone implZone = implementation.zone; + HandleUncaughtErrorHandler handler = implementation.function; + return handler(implZone, implZone._parentDelegate, zone, error, stackTrace); } R run(Zone zone, R f()) { @@ -1092,28 +1040,6 @@ abstract class _Zone implements Zone { return identical(this, otherZone) || identical(errorZone, otherZone.errorZone); } - - void _processUncaughtError(Zone zone, Object error, StackTrace stackTrace) { - var implementation = _handleUncaughtError; - _Zone implZone = implementation.zone; - if (identical(implZone, _rootZone)) { - _rootHandleError(error, stackTrace); - return; - } - HandleUncaughtErrorHandler handler = implementation.function; - ZoneDelegate parentDelegate = implZone._parentDelegate; - _Zone parentZone = implZone.parent!; // Not null for non-root zones. - _Zone currentZone = Zone._current; - try { - Zone._current = parentZone; - handler(implZone, parentDelegate, zone, error, stackTrace); - Zone._current = currentZone; - } catch (e, s) { - Zone._current = currentZone; - parentZone._processUncaughtError( - implZone, e, identical(error, e) ? stackTrace : s); - } - } } class _CustomZone extends _Zone { @@ -1309,7 +1235,11 @@ class _CustomZone extends _Zone { // Methods that can be customized by the zone specification. void handleUncaughtError(Object error, StackTrace stackTrace) { - _processUncaughtError(this, error, stackTrace); + var implementation = this._handleUncaughtError; + ZoneDelegate parentDelegate = implementation.zone._parentDelegate; + HandleUncaughtErrorHandler handler = implementation.function; + return handler( + implementation.zone, parentDelegate, this, error, stackTrace); } Zone fork( @@ -1405,10 +1335,6 @@ class _CustomZone extends _Zone { void _rootHandleUncaughtError(Zone? self, ZoneDelegate? parent, Zone zone, Object error, StackTrace stackTrace) { - _rootHandleError(error, stackTrace); -} - -void _rootHandleError(Object error, StackTrace stackTrace) { _schedulePriorityAsyncCallback(() { _rethrow(error, stackTrace); }); diff --git a/tests/lib/async/uncaught_error_handler_throws_test.dart b/tests/lib/async/uncaught_error_handler_throws_test.dart deleted file mode 100644 index f70f0987b29..00000000000 --- a/tests/lib/async/uncaught_error_handler_throws_test.dart +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright (c) 2021, 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. - -import 'package:expect/expect.dart'; -import 'package:async_helper/async_helper.dart'; -import 'dart:async'; - -void main() async { - asyncStart(); - await testThrowSame(); - await testThrowOther(); - asyncEnd(); -} - -Future testThrowSame() async { - asyncStart(); - var object1 = Object(); - var stack1 = StackTrace.current; - var outerZone = Zone.current; - var firstZone = Zone.current.fork(specification: onError((error, stack) { - // Uncaught error handlers run in the parent zone. - Expect.identical(outerZone, Zone.current); - Expect.identical(object1, error); - Expect.identical(stack1, stack); // Get same stack trace. - asyncEnd(); - })); - firstZone.run(() async { - Expect.identical(firstZone, Zone.current); - var secondZone = Zone.current.fork(specification: onError((error, stack) { - // Uncaught error handlers run in the parent zone. - Expect.identical(firstZone, Zone.current); - Expect.identical(object1, error); - Expect.identical(stack1, stack); - throw error; // Throw same object - })); - secondZone.run(() async { - Expect.identical(secondZone, Zone.current); - Future.error(object1, stack1); // Unhandled async error. - await Future(() {}); - }); - }); -} - -Future testThrowOther() async { - asyncStart(); - var object1 = Object(); - var object2 = Object(); - var stack1 = StackTrace.current; - var outerZone = Zone.current; - var firstZone = Zone.current.fork(specification: onError((error, stack) { - Expect.identical(outerZone, Zone.current); - Expect.identical(object2, error); - Expect.notIdentical(stack1, stack); // Get different stack trace. - asyncEnd(); - })); - firstZone.run(() async { - Expect.identical(firstZone, Zone.current); - var secondZone = Zone.current.fork(specification: onError((error, stack) { - Expect.identical(firstZone, Zone.current); - Expect.identical(object1, error); - Expect.identical(stack1, stack); - throw object2; // Throw different object - })); - secondZone.run(() async { - Expect.identical(secondZone, Zone.current); - Future.error(object1, stack1); // Unhandled async error. - await Future(() {}); - }); - }); -} - -ZoneSpecification onError(void Function(Object, StackTrace) handler) { - return ZoneSpecification( - handleUncaughtError: (s, p, z, e, st) => handler(e, st)); -} diff --git a/tests/lib_2/async/slow_consumer2_test.dart b/tests/lib_2/async/slow_consumer2_test.dart index 23a001ad1c7..de37668f6ba 100644 --- a/tests/lib_2/async/slow_consumer2_test.dart +++ b/tests/lib_2/async/slow_consumer2_test.dart @@ -88,7 +88,7 @@ class DataProvider { listSize -= sentCount - targetCount; sentCount = targetCount; } - controller.add(new List(listSize)); + controller.add(new List(listSize)); int ms = listSize * 1000 ~/ bytesPerSecond; Duration duration = new Duration(milliseconds: ms); if (!controller.isPaused) new Timer(duration, send); diff --git a/tests/lib_2/async/uncaught_error_handler_throws_test.dart b/tests/lib_2/async/uncaught_error_handler_throws_test.dart deleted file mode 100644 index f70f0987b29..00000000000 --- a/tests/lib_2/async/uncaught_error_handler_throws_test.dart +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright (c) 2021, 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. - -import 'package:expect/expect.dart'; -import 'package:async_helper/async_helper.dart'; -import 'dart:async'; - -void main() async { - asyncStart(); - await testThrowSame(); - await testThrowOther(); - asyncEnd(); -} - -Future testThrowSame() async { - asyncStart(); - var object1 = Object(); - var stack1 = StackTrace.current; - var outerZone = Zone.current; - var firstZone = Zone.current.fork(specification: onError((error, stack) { - // Uncaught error handlers run in the parent zone. - Expect.identical(outerZone, Zone.current); - Expect.identical(object1, error); - Expect.identical(stack1, stack); // Get same stack trace. - asyncEnd(); - })); - firstZone.run(() async { - Expect.identical(firstZone, Zone.current); - var secondZone = Zone.current.fork(specification: onError((error, stack) { - // Uncaught error handlers run in the parent zone. - Expect.identical(firstZone, Zone.current); - Expect.identical(object1, error); - Expect.identical(stack1, stack); - throw error; // Throw same object - })); - secondZone.run(() async { - Expect.identical(secondZone, Zone.current); - Future.error(object1, stack1); // Unhandled async error. - await Future(() {}); - }); - }); -} - -Future testThrowOther() async { - asyncStart(); - var object1 = Object(); - var object2 = Object(); - var stack1 = StackTrace.current; - var outerZone = Zone.current; - var firstZone = Zone.current.fork(specification: onError((error, stack) { - Expect.identical(outerZone, Zone.current); - Expect.identical(object2, error); - Expect.notIdentical(stack1, stack); // Get different stack trace. - asyncEnd(); - })); - firstZone.run(() async { - Expect.identical(firstZone, Zone.current); - var secondZone = Zone.current.fork(specification: onError((error, stack) { - Expect.identical(firstZone, Zone.current); - Expect.identical(object1, error); - Expect.identical(stack1, stack); - throw object2; // Throw different object - })); - secondZone.run(() async { - Expect.identical(secondZone, Zone.current); - Future.error(object1, stack1); // Unhandled async error. - await Future(() {}); - }); - }); -} - -ZoneSpecification onError(void Function(Object, StackTrace) handler) { - return ZoneSpecification( - handleUncaughtError: (s, p, z, e, st) => handler(e, st)); -}