languages/dart-sdk
languages/dart-sdk
1
0
Fork 0
mirror of https://github.com/dart-lang/sdk synced 2024-11-02 12:24:24 +00:00
Code Issues Projects Releases Packages Wiki Activity

[vm] Respect catchError attached to the last async frame.

Browse source 
Our async unwinding code used to only respect `catchError`
which occured between async frames and ignored trailing
`catchError` like in the code below

    Future(...).catchError((_) { /* handle error */ });

This CL also simplifies how we communicate the presence of the
exception handler to the debugger: the code in the debugger did
not actually care about which frame catches the error (for async
handlers), so we don't need to precisely mark async gaps with
`has_catch_error` flag. Instead we have a single boolean
produced by unwinding which signals whether there we encountered
an asynchronous error handler or not.

Fixes https://github.com/flutter/flutter/issues/141882

TEST=service/pause_on_unhandled_async_exceptions5

Change-Id: Id6f6a97ee5444c197b2c621f68d1e47082fc8997
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/350320
Commit-Queue: Slava Egorov <vegorov@google.com>
Reviewed-by: Alexander Aprelev <aam@google.com>
This commit is contained in:
Vyacheslav Egorov 2024-02-05 21:09:38 +00:00 committed by Commit Queue
parent 13bc0f57d6
commit 0ea5013250
6 changed files with 190 additions and 57 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

43
pkg/vm_service/test/common/service_test_common.dart
View file

@ -8,7 +8,7 @@ import 'dart:async';
import 'dart:developer';
import 'dart:typed_data';
import 'package:path/path.dart';
import 'package:path/path.dart' as p;
import 'package:test/test.dart';
import 'package:vm_service/vm_service.dart';
@ -241,6 +241,19 @@ IsolateTest stoppedAtLine(int line) {
' $f [${script.getLineNumberFromTokenPos(f.location!.tokenPos!)}]\n',
);
}
if (stack.asyncCausalFrames != null) {
final asyncFrames = stack.asyncCausalFrames!;
sb.write('\nFull async stack trace:\n');
for (Frame f in asyncFrames) {
sb.write(' $f');
if (f.location != null) {
sb.write(
' [${script.getLineNumberFromTokenPos(f.location!.tokenPos!)}]',
);
}
sb.writeln();
}
}
throw sb.toString();
} else {
print('Program is stopped at line: $line');
@ -378,7 +391,7 @@ Future<String> _locationToString(
final location = frame.location!;
final Script script =
await service.getObject(isolateRef.id!, location.script!.id!) as Script;
final scriptName = basename(script.uri!);
final scriptName = p.basename(script.uri!);
final tokenPos = location.tokenPos!;
final line = script.getLineNumberFromTokenPos(tokenPos);
final column = script.getColumnNumberFromTokenPos(tokenPos);
@ -710,3 +723,29 @@ IsolateTest stoppedInFunction(String functionName) {
}
};
}
Future<void> expectFrame(
VmService service,
IsolateRef isolate,
Frame frame, {
String kind = 'Regular',
String? functionName,
int? line,
}) async {
expect(frame.kind, equals(kind));
if (functionName != null) {
expect(frame.function?.name, equals(functionName));
}
if (line != null) {
expect(frame.location, isNotNull);
final script = await service.getObject(
isolate.id!,
frame.location!.script!.id!,
) as Script;
expect(
script.getLineNumberFromTokenPos(frame.location!.tokenPos!),
equals(line),
);
}
}

99
pkg/vm_service/test/pause_on_unhandled_async_exceptions5_test.dart Normal file
View file

@ -0,0 +1,99 @@
// Copyright (c) 2024, 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.
// Verifies that debugger correctly detects `catchError` even when it occurs
// without a value listener, as in `Future(...).catchError(...)`.
//
// Regression test for https://github.com/flutter/flutter/issues/141882.
import 'dart:async';
import 'package:vm_service/vm_service.dart';
import 'common/service_test_common.dart';
import 'common/test_helper.dart';
// AUTOGENERATED START
//
// Update these constants by running:
//
// dart runtime/observatory/tests/service/update_line_numbers.dart <test.dart>
//
const int LINE_A = 29;
const int LINE_B = 33;
const int LINE_C = 38;
// AUTOGENERATED END
Future<void> doThrowAsync() async {
await null;
throw 'Exception'; // LINE_A
}
Future<void> doThrowSync() async {
throw 'Exception'; // LINE_B
}
Future<void> testeeMain() async {
await testCatchErrorVariants();
await doThrowAsync(); // LINE_C
}
Future<void> testCatchErrorVariants() async {
await testCatchError(doThrowSync);
await testCatchError(doThrowAsync);
await testCatchError(() async {
await doThrowSync();
});
await testCatchError(() async {
await doThrowAsync();
});
}
Future<void> testCatchError(void Function() f) async {
await Future<void>(f).catchError((_) {});
final c = Completer<void>();
Future(f).catchError(c.completeError); // ignore:unawaited_futures
try {
await c.future;
} catch (_) {}
}
var tests = <IsolateTest>[
hasStoppedWithUnhandledException,
stoppedAtLine(LINE_A),
(VmService service, IsolateRef isolate) async {
final stack = await service.getStack(isolate.id!);
final frames = stack.asyncCausalFrames!;
await expectFrame(
service,
isolate,
frames[0],
functionName: 'doThrowAsync',
line: LINE_A,
);
await expectFrame(
service,
isolate,
frames[1],
kind: 'AsyncSuspensionMarker',
);
await expectFrame(
service,
isolate,
frames[2],
kind: 'AsyncCausal',
functionName: 'testeeMain',
line: LINE_C,
);
}
];
void main([args = const <String>[]]) => runIsolateTests(
args,
tests,
'pause_on_unhandled_async_exceptions5_test.dart',
pauseOnUnhandledExceptions: true,
testeeConcurrent: testeeMain,
extraArgs: extraDebuggingArgs,
);

20
runtime/vm/debugger.cc
View file

@ -656,7 +656,7 @@ intptr_t ActivationFrame::ContextLevel() {
bool ActivationFrame::HandlesException(const Instance& exc_obj) {
if (kind_ == kAsyncSuspensionMarker) {
return has_catch_error();
return false;
}
intptr_t try_index = TryIndex();
const auto& handlers = ExceptionHandlers::Handle(code().exception_handlers());
@ -1288,7 +1288,7 @@ void DebuggerStackTrace::AddActivation(ActivationFrame* frame) {
}
}
void DebuggerStackTrace::AddAsyncSuspension(bool has_catch_error) {
void DebuggerStackTrace::AddAsyncSuspension() {
// We might start asynchronous unwinding in one of the internal
// dart:async functions which would make synchronous part of the
// stack empty. This would not happen normally but might happen
@ -1297,9 +1297,6 @@ void DebuggerStackTrace::AddAsyncSuspension(bool has_catch_error) {
trace_.Last()->kind() != ActivationFrame::kAsyncSuspensionMarker) {
trace_.Add(new ActivationFrame(ActivationFrame::kAsyncSuspensionMarker));
}
if (has_catch_error) {
trace_.Last()->set_has_catch_error(true);
}
}
void DebuggerStackTrace::AddAsyncAwaiterFrame(uword pc,
@ -1737,15 +1734,17 @@ DebuggerStackTrace* DebuggerStackTrace::CollectAsyncAwaiters() {
auto stack_trace = new DebuggerStackTrace(kDefaultStackAllocation);
bool has_async = false;
bool has_async_catch_error = false;
StackTraceUtils::CollectFrames(
thread, /*skip_frames=*/0, [&](const StackTraceUtils::Frame& frame) {
thread, /*skip_frames=*/0,
[&](const StackTraceUtils::Frame& frame) {
if (frame.frame != nullptr) { // Synchronous portion of the stack.
stack_trace->AppendCodeFrames(frame.frame, frame.code);
} else {
has_async = true;
if (frame.code.ptr() == StubCode::AsynchronousGapMarker().ptr()) {
stack_trace->AddAsyncSuspension(frame.has_async_catch_error);
stack_trace->AddAsyncSuspension();
return;
}
@ -1759,13 +1758,16 @@ DebuggerStackTrace* DebuggerStackTrace::CollectAsyncAwaiters() {
stack_trace->AddAsyncAwaiterFrame(absolute_pc, frame.code,
frame.closure);
}
});
},
&has_async_catch_error);
// If the entire stack is sync, return no (async) trace.
if (!has_async) {
return nullptr;
}
stack_trace->set_has_async_catch_error(has_async_catch_error);
return stack_trace;
}
@ -1882,7 +1884,7 @@ bool Debugger::ShouldPauseOnException(DebuggerStackTrace* stack_trace,
// uninstantiated types, i.e. types containing type parameters.
// Thus, we may report an exception as unhandled when in fact
// it will be caught once we unwind the stack.
return true;
return !stack_trace->has_async_catch_error();
}
auto& handler_function = Function::Handle(handler_frame->function().ptr());

13
runtime/vm/debugger.h
View file

@ -385,9 +385,6 @@ class ActivationFrame : public ZoneAllocated {
bool HandlesException(const Instance& exc_obj);
bool has_catch_error() const { return has_catch_error_; }
void set_has_catch_error(bool value) { has_catch_error_ = value; }
private:
void PrintToJSONObjectRegular(JSONObject* jsobj);
void PrintToJSONObjectAsyncAwaiter(JSONObject* jsobj);
@ -454,8 +451,6 @@ class ActivationFrame : public ZoneAllocated {
ZoneGrowableArray<intptr_t> desc_indices_;
PcDescriptors& pc_desc_ = PcDescriptors::ZoneHandle();
bool has_catch_error_ = false;
friend class Debugger;
friend class DebuggerStackTrace;
DISALLOW_COPY_AND_ASSIGN(ActivationFrame);
@ -481,9 +476,14 @@ class DebuggerStackTrace : public ZoneAllocated {
// to query local variables in the returned stack.
static DebuggerStackTrace* From(const class StackTrace& ex_trace);
bool has_async_catch_error() const { return has_async_catch_error_; }
void set_has_async_catch_error(bool has_async_catch_error) {
has_async_catch_error_ = has_async_catch_error;
}
private:
void AddActivation(ActivationFrame* frame);
void AddAsyncSuspension(bool has_catch_error);
void AddAsyncSuspension();
void AddAsyncAwaiterFrame(uword pc, const Code& code, const Closure& closure);
void AppendCodeFrames(StackFrame* frame, const Code& code);
@ -493,6 +493,7 @@ class DebuggerStackTrace : public ZoneAllocated {
Code& inlined_code_ = Code::Handle();
Array& deopt_frame_ = Array::Handle();
ZoneGrowableArray<ActivationFrame*> trace_;
bool has_async_catch_error_ = false;
friend class Debugger;

65
runtime/vm/stack_trace.cc
View file

@ -68,9 +68,13 @@ class AsyncAwareStackUnwinder : public ValueObject {
async_lib_(Library::Handle(zone_, Library::AsyncLibrary())),
null_closure_(Closure::Handle(zone_)) {}
bool Unwind(intptr_t skip_frames,
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();
@ -168,7 +172,6 @@ class AsyncAwareStackUnwinder : public ValueObject {
struct AwaiterFrame {
Closure& closure;
Object& next;
bool has_catch_error;
};
Zone* zone_;
@ -177,6 +180,8 @@ class AsyncAwareStackUnwinder : public ValueObject {
StackFrame* sync_frame_;
AwaiterFrame awaiter_frame_;
bool encountered_async_catch_error_ = false;
Closure& closure_;
Code& code_;
Context& context_;
@ -198,7 +203,7 @@ class AsyncAwareStackUnwinder : public ValueObject {
DISALLOW_COPY_AND_ASSIGN(AsyncAwareStackUnwinder);
};
bool AsyncAwareStackUnwinder::Unwind(
void AsyncAwareStackUnwinder::Unwind(
intptr_t skip_frames,
std::function<void(const StackTraceUtils::Frame&)> handle_frame) {
// First skip the given number of synchronous frames.
@ -215,11 +220,15 @@ bool AsyncAwareStackUnwinder::Unwind(
if (!was_handled) {
code_ = sync_frame_->LookupDartCode();
const uword pc_offset = sync_frame_->pc() - code_.PayloadStart();
handle_frame({sync_frame_, code_, pc_offset, null_closure_, false});
handle_frame({sync_frame_, code_, pc_offset, null_closure_});
}
sync_frame_ = sync_frames_.NextFrame();
}
const StackTraceUtils::Frame gap_frame = {nullptr,
StubCode::AsynchronousGapMarker(),
/*pc_offset=*/0, null_closure_};
// Traverse awaiter frames.
bool any_async = false;
for (; !awaiter_frame_.closure.IsNull(); UnwindToAwaiter()) {
@ -229,6 +238,7 @@ bool AsyncAwareStackUnwinder::Unwind(
}
any_async = true;
uword pc_offset;
if (awaiter_frame_.next.IsSuspendState()) {
const uword pc = SuspendState::Cast(awaiter_frame_.next).pc();
if (pc == 0) {
@ -237,23 +247,25 @@ bool AsyncAwareStackUnwinder::Unwind(
}
code_ = SuspendState::Cast(awaiter_frame_.next).GetCodeObject();
const uword pc_offset = pc - code_.PayloadStart();
handle_frame({nullptr, code_, pc_offset, awaiter_frame_.closure,
awaiter_frame_.has_catch_error});
pc_offset = pc - code_.PayloadStart();
} else {
// This is an asynchronous continuation represented by a closure which
// will handle successful completion. This function is not yet executing
// so we have to use artificial marker offset (1).
code_ = function_.EnsureHasCode();
RELEASE_ASSERT(!code_.IsNull());
const uword pc_offset =
pc_offset =
(function_.entry_point() + StackTraceUtils::kFutureListenerPcOffset) -
code_.PayloadStart();
handle_frame({nullptr, code_, pc_offset, awaiter_frame_.closure,
awaiter_frame_.has_catch_error});
}
handle_frame(gap_frame);
handle_frame({nullptr, code_, pc_offset, awaiter_frame_.closure});
}
if (any_async) {
handle_frame(gap_frame);
}
return any_async;
}
ObjectPtr AsyncAwareStackUnwinder::GetReceiver() const {
@ -314,7 +326,6 @@ void AsyncAwareStackUnwinder::InitializeAwaiterFrameFromSuspendState() {
}
void AsyncAwareStackUnwinder::UnwindToAwaiter() {
awaiter_frame_.has_catch_error = false;
do {
UnwindAwaiterFrame();
} while (awaiter_frame_.closure.IsNull() && !awaiter_frame_.next.IsNull());
@ -399,7 +410,7 @@ void AsyncAwareStackUnwinder::InitializeAwaiterFrameFromFutureListener(
Closure::RawCast(Get_FutureListener_callback(listener));
// If the Future has catchError callback attached through either
// `Future.onError` or `Future.then(..., onError: ...)` then we should
// `Future.catchError` or `Future.then(..., onError: ...)` then we should
// treat this listener as a catch all exception handler. However we should
// ignore the case when these callbacks are simply forwarding errors into a
// suspended async function, because it will be represented by its own async
@ -407,7 +418,7 @@ void AsyncAwareStackUnwinder::InitializeAwaiterFrameFromFutureListener(
if ((state &
(k_FutureListener_stateCatchError | k_FutureListener_maskAwait)) ==
k_FutureListener_stateCatchError) {
awaiter_frame_.has_catch_error = true;
encountered_async_catch_error_ = true;
}
}
@ -494,28 +505,12 @@ bool StackTraceUtils::IsNeededForAsyncAwareUnwinding(const Function& function) {
void StackTraceUtils::CollectFrames(
Thread* thread,
int skip_frames,
const std::function<void(const StackTraceUtils::Frame&)>& handle_frame) {
const Closure& null_closure = Closure::Handle(thread->zone());
const Frame gap_frame = {nullptr, StubCode::AsynchronousGapMarker(),
/*pc_offset=*/0, null_closure, false};
const Frame gap_frame_with_catch = {nullptr,
StubCode::AsynchronousGapMarker(),
/*pc_offset=*/0, null_closure, true};
const std::function<void(const StackTraceUtils::Frame&)>& handle_frame,
bool* has_async_catch_error /* = null */) {
AsyncAwareStackUnwinder it(thread);
const bool any_async = it.Unwind(skip_frames, [&](const Frame& frame) {
if (frame.frame == nullptr) {
handle_frame(frame.has_async_catch_error ? gap_frame_with_catch
: gap_frame);
}
handle_frame(frame);
});
if (any_async) {
handle_frame(gap_frame);
it.Unwind(skip_frames, handle_frame);
if (has_async_catch_error != nullptr) {
*has_async_catch_error = it.encountered_async_catch_error();
}
}

7
runtime/vm/stack_trace.h
View file

@ -35,10 +35,6 @@ class StackTraceUtils : public AllStatic {
// Closure corresponding to the awaiter frame or |null| if this is
// a synchronous frame or a gap.
const Closure& closure;
// |true| if an asynchronous exception would be caught by a |catchError|
// listener somewhere between the previous frame and this frame.
bool has_async_catch_error;
};
// Returns |true| if this function is needed to correctly unwind through
@ -55,7 +51,8 @@ class StackTraceUtils : public AllStatic {
static void CollectFrames(
Thread* thread,
int skip_frames,
const std::function<void(const Frame&)>& handle_frame);
const std::function<void(const Frame&)>& handle_frame,
bool* has_async_catch_error = nullptr);
// If |closure| has an awaiter-link pointing to the |SuspendState|
// the return that object.