dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

[vm] Improve detection of handled async errors

Browse Source 
Improve the handling of code where the async flow forks:
e.g. for `fut.whenComplete(c.complete)` we might proceed
unwinding through awaiters of `c.future` and forget about the
future returned from `whenComplete`. This happens because
we choose to present result of the unwinding as a single
stack and not a tree. In this situation the error will only
propagate into that future and whether or not the error will be
handled depends on that future alone.

As part of this change also start respecting ignored bit on
futures without listeners.

TEST=pkg/vm_service/test/pause_on_unhandled_async_exceptions7_test.dart

CoreLibraryReviewExempt: Comment only changes to core library
Change-Id: I27b689ab07a725e8faa8d91cf40e88ebc8c441a0
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/352904
Reviewed-by: Martin Kustermann <kustermann@google.com>
Reviewed-by: Lasse Nielsen <lrn@google.com>
Commit-Queue: Slava Egorov <vegorov@google.com>
This commit is contained in:
Vyacheslav Egorov
2024-02-16 12:55:22 +00:00
committed by Commit Queue
parent 2c11ad08bb
commit 2280b06202
4 changed files with 119 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/vm_service/test/pause_on_unhandled_async_exceptions7_test.dart
+19 -1
View File
@@ -20,7 +20,7 @@ import 'pause_on_unhandled_async_exceptions6_test.dart' as test6;
Future<int> alwaysThrow() async {
// Ensure that we suspend at least once and throw an error asynchronously.
await Future.delayed(const Duration(milliseconds: 10));
await Future.delayed(Duration.zero);
print(StackTrace.current);
throw 'Error';
}
@@ -70,6 +70,24 @@ Future<void> throwSomeCaughtAsyncErrors() async {
} finally {
client.close();
}
{
print('ignoring an error (1)');
final c = Completer<void>();
// Here async unwinder might follow awaiter chain to `await c.future`
// because it detects forwarding automatically.
alwaysThrow().whenComplete(c.complete).ignore();
await c.future;
}
{
print('ignoring an error (2)');
final c = Completer<void>();
alwaysThrow().whenComplete(() {
c.complete();
}).ignore();
await c.future;
}
}
Future<void> testeeMain() async {
runtime/vm/stack_trace.cc
+95 -17
View File
@@ -22,6 +22,8 @@ const intptr_t k_StreamController__STATE_SUBSCRIPTION_MASK = 3;
const intptr_t k_StreamController__STATE_ADDSTREAM = 8;
// - _BufferingStreamSubscription._STATE_HAS_ERROR_HANDLER.
const intptr_t k_BufferingStreamSubscription__STATE_HAS_ERROR_HANDLER = 1 << 5;
// - _Future._stateIgnoreError
const intptr_t k_Future__stateIgnoreError = 1;
// - _FutureListener.stateThen.
const intptr_t k_FutureListener_stateThen = 1;
// - _FutureListener.stateCatchError.
@@ -79,31 +81,34 @@ bool TryGetAwaiterLink(const Closure& closure, Object* link) {
// of awaiters.
class AsyncAwareStackUnwinder : public ValueObject {
public:
explicit AsyncAwareStackUnwinder(Thread* thread)
explicit AsyncAwareStackUnwinder(Thread* thread,
bool* encountered_async_catch_error)
: zone_(thread->zone()),
sync_frames_(thread, StackFrameIterator::kNoCrossThreadIteration),
sync_frame_(nullptr),
awaiter_frame_{Closure::Handle(zone_), Object::Handle(zone_)},
encountered_async_catch_error_(encountered_async_catch_error),
closure_(Closure::Handle(zone_)),
code_(Code::Handle(zone_)),
context_(Context::Handle(zone_)),
function_(Function::Handle(zone_)),
parent_function_(Function::Handle(zone_)),
object_(Object::Handle(zone_)),
result_future_(Object::Handle(zone_)),
suspend_state_(SuspendState::Handle(zone_)),
controller_(Object::Handle(zone_)),
subscription_(Object::Handle(zone_)),
stream_iterator_(Object::Handle(zone_)),
async_lib_(Library::Handle(zone_, Library::AsyncLibrary())),
null_closure_(Closure::Handle(zone_)) {}
null_closure_(Closure::Handle(zone_)) {
if (encountered_async_catch_error_ != nullptr) {
*encountered_async_catch_error_ = false;
}
}
void Unwind(intptr_t skip_frames,
std::function<void(const StackTraceUtils::Frame&)> handle_frame);
bool encountered_async_catch_error() const {
return encountered_async_catch_error_;
}
private:
bool HandleSynchronousFrame();
@@ -137,6 +142,28 @@ class AsyncAwareStackUnwinder : public ValueObject {
ObjectPtr GetReceiver() const;
// Returns |true| if propagating an error to the listeners of this `_Future`
// will always encounter an error handler. Future handles error iff:
//
// * It has no listeners and is marked as ignoring errors
// * All of its listeners either have an error handler or corresponding
// result future handles error.
//
// Note: this ignores simple error forwarding/rethrow which occurs in `await`
// or patterns like `fut.then(onError: c.completeError)`.
bool WillFutureHandleError(const Object& future, intptr_t depth = 0);
void MarkAsHandlingAsyncError() const {
if (ShouldComputeIfAsyncErrorIsHandled()) {
*encountered_async_catch_error_ = true;
}
}
bool ShouldComputeIfAsyncErrorIsHandled() const {
return encountered_async_catch_error_ != nullptr &&
!*encountered_async_catch_error_;
}
#define USED_CLASS_LIST(V) \
V(_AsyncStarStreamController) \
V(_BufferingStreamSubscription) \
@@ -167,9 +194,11 @@ class AsyncAwareStackUnwinder : public ValueObject {
V(_BufferingStreamSubscription, _state) \
V(_Completer, future) \
V(_Future, _resultOrListeners) \
V(_Future, _state) \
V(_FutureListener, callback) \
V(_FutureListener, result) \
V(_FutureListener, state) \
V(_FutureListener, _nextListener) \
V(_StreamController, _state) \
V(_StreamController, _varData) \
V(_StreamControllerAddStreamState, _varData) \
@@ -226,7 +255,7 @@ class AsyncAwareStackUnwinder : public ValueObject {
StackFrame* sync_frame_;
AwaiterFrame awaiter_frame_;
bool encountered_async_catch_error_ = false;
bool* encountered_async_catch_error_;
Closure& closure_;
Code& code_;
@@ -234,6 +263,7 @@ class AsyncAwareStackUnwinder : public ValueObject {
Function& function_;
Function& parent_function_;
Object& object_;
Object& result_future_;
SuspendState& suspend_state_;
Object& controller_;
@@ -423,11 +453,47 @@ void AsyncAwareStackUnwinder::UnwindFrameToFutureListener() {
if (object_.GetClassId() == _FutureListener().id()) {
InitializeAwaiterFrameFromFutureListener(object_);
} else {
if (ShouldComputeIfAsyncErrorIsHandled()) {
// Check if error on this future was ignored through |Future.ignore|.
const auto state =
Smi::Value(Smi::RawCast(Get_Future__state(awaiter_frame_.next)));
if ((state & k_Future__stateIgnoreError) != 0) {
MarkAsHandlingAsyncError();
}
}
awaiter_frame_.closure = Closure::null();
awaiter_frame_.next = Object::null();
}
}
bool AsyncAwareStackUnwinder::WillFutureHandleError(const Object& future,
intptr_t depth /* = 0 */) {
if (depth > 100 || future.GetClassId() != _Future().id()) {
return true; // Conservative.
}
if (Get_Future__resultOrListeners(future) == Object::null()) {
// No listeners: check if future is simply ignoring errors.
const auto state = Smi::Value(Smi::RawCast(Get_Future__state(future)));
return (state & k_Future__stateIgnoreError) != 0;
}
for (auto& listener = Object::Handle(Get_Future__resultOrListeners(future));
listener.GetClassId() == _FutureListener().id();
listener = Get_FutureListener__nextListener(listener)) {
const auto state =
Smi::Value(Smi::RawCast(Get_FutureListener_state(listener)));
if ((state & k_FutureListener_stateCatchError) == 0 &&
!WillFutureHandleError(
Object::Handle(Get_FutureListener_result(listener)), depth + 1)) {
return false;
}
}
return true;
}
void AsyncAwareStackUnwinder::InitializeAwaiterFrameFromFutureListener(
const Object& listener) {
if (listener.GetClassId() != _FutureListener().id()) {
@@ -445,15 +511,30 @@ void AsyncAwareStackUnwinder::InitializeAwaiterFrameFromFutureListener(
(k_FutureListener_stateThen | k_FutureListener_stateCatchError) ||
state == (k_FutureListener_stateThen | k_FutureListener_stateCatchError |
k_FutureListener_maskAwait)) {
awaiter_frame_.next = Get_FutureListener_result(listener);
result_future_ = Get_FutureListener_result(listener);
} else {
awaiter_frame_.next = Object::null();
result_future_ = Object::null();
}
awaiter_frame_.closure =
Closure::RawCast(Get_FutureListener_callback(listener));
awaiter_frame_.next = result_future_.ptr();
ComputeNextFrameFromAwaiterLink();
if (ShouldComputeIfAsyncErrorIsHandled() && !result_future_.IsNull() &&
result_future_.ptr() != awaiter_frame_.next.ptr()) {
// We have unwound through closure rather than followed result future, this
// can be caused by unwinding through `await` or code like
// `fut.whenComplete(c.complete)`. If the current future does not
// catch the error then the error will be forwarded into result_future_.
// Check if result_future_ handles it and set
// encountered_async_catch_error_ respectively.
if ((state & k_FutureListener_stateCatchError) == 0 &&
WillFutureHandleError(result_future_)) {
MarkAsHandlingAsyncError();
}
}
// If the Future has catchError callback attached through either
// `Future.catchError` or `Future.then(..., onError: ...)` then we should
// treat this listener as a catch all exception handler. However we should
@@ -468,7 +549,7 @@ void AsyncAwareStackUnwinder::InitializeAwaiterFrameFromFutureListener(
if ((state & k_FutureListener_stateCatchError) != 0 &&
((state & k_FutureListener_maskAwait) == 0 ||
!awaiter_frame_.next.IsSuspendState())) {
encountered_async_catch_error_ = true;
MarkAsHandlingAsyncError();
}
}
@@ -531,7 +612,7 @@ void AsyncAwareStackUnwinder::UnwindFrameToStreamController() {
if (Get_StreamIterator__hasValue(stream_iterator_) ==
Object::bool_true().ptr()) {
if (has_error_handler) {
encountered_async_catch_error_ = true;
MarkAsHandlingAsyncError();
}
return;
}
@@ -546,7 +627,7 @@ void AsyncAwareStackUnwinder::UnwindFrameToStreamController() {
awaiter_frame_.next = object_.ptr();
} else {
if (has_error_handler) {
encountered_async_catch_error_ = true;
MarkAsHandlingAsyncError();
}
}
return;
@@ -611,7 +692,7 @@ void AsyncAwareStackUnwinder::UnwindFrameToStreamController() {
if (has_error_handler && object_.GetClassId() != _Future().id() &&
object_.GetClassId() != _SyncStreamController().id()) {
encountered_async_catch_error_ = true;
MarkAsHandlingAsyncError();
}
if (found_awaiter_link_in_sibling_handler) {
@@ -666,11 +747,8 @@ void StackTraceUtils::CollectFrames(
int skip_frames,
const std::function<void(const StackTraceUtils::Frame&)>& handle_frame,
bool* has_async_catch_error /* = null */) {
AsyncAwareStackUnwinder it(thread);
AsyncAwareStackUnwinder it(thread, has_async_catch_error);
it.Unwind(skip_frames, handle_frame);
if (has_async_catch_error != nullptr) {
*has_async_catch_error = it.encountered_async_catch_error();
}
}
bool StackTraceUtils::GetSuspendState(const Closure& closure,
runtime/vm/symbols.h
+1
View File
@@ -454,6 +454,7 @@ class ObjectPointerVisitor;
V(_mapGet, "_mapGet") \
V(_mapKeys, "_mapKeys") \
V(_name, "_name") \
V(_nextListener, "_nextListener") \
V(_nativeGetFloat32, "_nativeGetFloat32") \
V(_nativeSetFloat32, "_nativeSetFloat32") \
V(_nativeGetFloat64, "_nativeGetFloat64") \
sdk/lib/async/future_impl.dart
+4
View File
@@ -88,6 +88,7 @@ class _FutureListener<S, T> {
maskValue | maskError | maskTestError | maskWhenComplete;
// Listeners on the same future are linked through this link.
@pragma("vm:entry-point")
_FutureListener? _nextListener;
// The future to complete when this listener is activated.
@@ -223,6 +224,8 @@ class _Future<T> implements Future<T> {
/// Set by the [FutureExtensions.ignore] method to avoid
/// having to introduce an unnecessary listener.
/// Only relevant until the future is completed.
///
/// When changing update runtime/vm/stack_trace.cc
static const int _stateIgnoreError = 1;
/// Pending completion. Set when completed using [_asyncComplete] or
@@ -254,6 +257,7 @@ class _Future<T> implements Future<T> {
static const int _completionStateMask = 30;
/// Whether the future is complete, and as what.
@pragma('vm:entry-point')
int _state = _stateIncomplete;
/// Zone that the future was completed from.