Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit da1e42a

Browse files
Clement Skaucommit-bot@chromium.org
Clement Skau
authored and
[email protected]
committed
[SDK] Fixes async* stack unwinding.
For `foo() async* {}` frames we find the "caller" by finding out what closure is registered as listener on the _AsyncStreamController. There can be two cases: a) The caller does a regular `foo().listen((_) {})`: The stack trace will have the closure as the caller and unwinding stops. b) The caller uses 'await for (... foo())': In this case the listener will be a StreamIterator. This CL changes our unwinding code to get the awaiter of `await it.moveNext()`. Bug: dart-lang/sdk#39525 Change-Id: I13f76025a15682aaf55fd968088fc2059982d842 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/132841 Commit-Queue: Clement Skau <[email protected]> Reviewed-by: Martin Kustermann <[email protected]>
1 parent bc93488 commit da1e42a

File tree

6 files changed

+205
-42
lines changed

6 files changed

+205
-42
lines changed

‎runtime/observatory/tests/service/causal_async_star_stack_contents_test.dart

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ var tests = <IsolateTest>[
5252
} else {
5353
expect(asyncStack.length, greaterThanOrEqualTo(1));
5454
expect(asyncStack[0].toString(), contains('helper'));
55+
// helper isn't awaited.
5556
}
5657
},
5758
resumeIsolate,
@@ -62,17 +63,14 @@ var tests = <IsolateTest>[
6263
// Has causal frames (we are inside an async function)
6364
expect(stack['asyncCausalFrames'], isNotNull);
6465
var asyncStack = stack['asyncCausalFrames'];
66+
expect(asyncStack.length, greaterThanOrEqualTo(3));
67+
expect(asyncStack[0].toString(), contains('foobar'));
68+
expect(asyncStack[1].kind, equals(M.FrameKind.asyncSuspensionMarker));
69+
expect(asyncStack[2].toString(), contains('helper'));
70+
expect(asyncStack[3].kind, equals(M.FrameKind.asyncSuspensionMarker));
6571
if (useCausalAsyncStacks) {
66-
expect(asyncStack.length, greaterThanOrEqualTo(3));
67-
expect(asyncStack[0].toString(), contains('foobar'));
68-
expect(asyncStack[1].kind, equals(M.FrameKind.asyncSuspensionMarker));
69-
expect(asyncStack[2].toString(), contains('helper'));
70-
expect(asyncStack[3].kind, equals(M.FrameKind.asyncSuspensionMarker));
72+
// helper isn't awaited.
7173
expect(asyncStack[4].toString(), contains('testMain'));
72-
} else {
73-
// TODO(dartbug.com/37668): Implement suport for this in the debugger.
74-
expect(asyncStack.length, greaterThanOrEqualTo(1));
75-
expect(asyncStack[0].toString(), contains('foobar'));
7674
}
7775
},
7876
resumeIsolate,
@@ -87,22 +85,19 @@ var tests = <IsolateTest>[
8785
await printFrames(asyncStack);
8886
print('sync:');
8987
await printFrames(stack['frames']);
88+
expect(asyncStack.length, greaterThanOrEqualTo(4));
89+
expect(asyncStack[0].toString(), contains('foobar'));
90+
expect(
91+
await asyncStack[0].location.toUserString(), contains('.dart:$LINE_C'));
92+
expect(asyncStack[1].kind, equals(M.FrameKind.asyncSuspensionMarker));
93+
expect(asyncStack[2].toString(), contains('helper'));
94+
expect(await asyncStack[2].location.toUserString(), contains('.dart:30'));
95+
expect(asyncStack[3].kind, equals(M.FrameKind.asyncSuspensionMarker));
9096
if (useCausalAsyncStacks) {
97+
// helper isn't awaited.
9198
expect(asyncStack.length, greaterThanOrEqualTo(5));
92-
expect(asyncStack[0].toString(), contains('foobar'));
93-
expect(asyncStack[1].kind, equals(M.FrameKind.asyncSuspensionMarker));
94-
expect(asyncStack[2].toString(), contains('helper'));
95-
expect(asyncStack[3].kind, equals(M.FrameKind.asyncSuspensionMarker));
9699
expect(asyncStack[4].toString(), contains('testMain'));
97-
expect(await asyncStack[0].location.toUserString(),
98-
contains('.dart:$LINE_C'));
99-
expect(await asyncStack[2].location.toUserString(), contains('.dart:30'));
100100
expect(await asyncStack[4].location.toUserString(), contains('.dart:36'));
101-
} else {
102-
// TODO(dartbug.com/37668): Implement suport for this in the debugger.
103-
expect(asyncStack.length, greaterThanOrEqualTo(2));
104-
expect(asyncStack[0].toString(), contains('foobar'));
105-
expect(asyncStack[1].kind, equals(M.FrameKind.asyncSuspensionMarker));
106101
}
107102
},
108103
];

‎runtime/tests/vm/dart/causal_stacks/utils.dart

Lines changed: 133 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,12 @@ Stream<Future> asyncStarThrowAsync() async* {
127127
}
128128
}
129129

130+
Future listenAsyncStarThrowAsync() async {
131+
// Listening to an async* doesn't create the usual await-for StreamIterator.
132+
StreamSubscription ss = asyncStarThrowAsync().listen((Future f) {});
133+
await ss.asFuture();
134+
}
135+
130136
// Helpers:
131137

132138
void assertStack(List<String> expects, StackTrace stackTrace) {
@@ -472,6 +478,50 @@ Future<void> doTestsCausal() async {
472478
r'^#7 _RawReceivePortImpl._handleMessage \(.+\)$',
473479
r'^$',
474480
]);
481+
482+
final listenAsyncStartExpected = const <String>[
483+
r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$',
484+
r'^<asynchronous suspension>$',
485+
r'^#1 asyncStarThrowAsync \(.*/utils.dart:126(:11)?\)$',
486+
r'^<asynchronous suspension>$',
487+
r'^#2 listenAsyncStarThrowAsync \(.+/utils.dart:132(:27)?\)$',
488+
];
489+
await doTestAwait(
490+
listenAsyncStarThrowAsync,
491+
listenAsyncStartExpected +
492+
const <String>[
493+
r'^#3 doTestAwait \(.+\)$',
494+
r'^#4 doTestsCausal \(.+\)$',
495+
r'^<asynchronous suspension>$',
496+
r'^#5 main \(.+\)$',
497+
r'^#6 _startIsolate.<anonymous closure> \(.+\)$',
498+
r'^#7 _RawReceivePortImpl._handleMessage \(.+\)$',
499+
r'^$',
500+
]);
501+
await doTestAwaitThen(
502+
listenAsyncStarThrowAsync,
503+
listenAsyncStartExpected +
504+
const <String>[
505+
r'^#3 doTestAwaitThen \(.+\)$',
506+
r'^#4 doTestsCausal \(.+\)$',
507+
r'^<asynchronous suspension>$',
508+
r'^#5 main \(.+\)$',
509+
r'^#6 _startIsolate.<anonymous closure> \(.+\)$',
510+
r'^#7 _RawReceivePortImpl._handleMessage \(.+\)$',
511+
r'^$',
512+
]);
513+
await doTestAwaitCatchError(
514+
listenAsyncStarThrowAsync,
515+
listenAsyncStartExpected +
516+
const <String>[
517+
r'^#3 doTestAwaitCatchError \(.+\)$',
518+
r'^#4 doTestsCausal \(.+\)$',
519+
r'^<asynchronous suspension>$',
520+
r'^#5 main \(.+\)$',
521+
r'^#6 _startIsolate.<anonymous closure> \(.+\)$',
522+
r'^#7 _RawReceivePortImpl._handleMessage \(.+\)$',
523+
r'^$',
524+
]);
475525
}
476526

477527
// For: --no-causal-async-stacks
@@ -745,6 +795,25 @@ Future<void> doTestsNoCausal() async {
745795
awaitEveryAsyncStarThrowAsync, asyncStarThrowAsyncExpected);
746796
await doTestAwaitCatchError(
747797
awaitEveryAsyncStarThrowAsync, asyncStarThrowAsyncExpected);
798+
799+
final listenAsyncStartExpected = const <String>[
800+
r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$',
801+
r'^#1 _RootZone.runUnary ',
802+
r'^#2 _FutureListener.handleValue ',
803+
r'^#3 Future._propagateToListeners.handleValueCallback ',
804+
r'^#4 Future._propagateToListeners ',
805+
// TODO(dart-vm): Figure out why this is inconsistent:
806+
r'^#5 Future.(_addListener|_prependListeners).<anonymous closure> ',
807+
r'^#6 _microtaskLoop ',
808+
r'^#7 _startMicrotaskLoop ',
809+
r'^#8 _runPendingImmediateCallback ',
810+
r'^#9 _RawReceivePortImpl._handleMessage ',
811+
r'^$',
812+
];
813+
await doTestAwait(listenAsyncStarThrowAsync, listenAsyncStartExpected);
814+
await doTestAwaitThen(listenAsyncStarThrowAsync, listenAsyncStartExpected);
815+
await doTestAwaitCatchError(
816+
listenAsyncStarThrowAsync, listenAsyncStartExpected);
748817
}
749818

750819
// For: --lazy-async-stacks
@@ -947,27 +1016,82 @@ Future<void> doTestsLazy() async {
9471016
r'^#0 throwSync \(.+/utils.dart:16(:3)?\)$',
9481017
r'^#1 asyncStarThrowSync \(.+/utils.dart:112(:11)?\)$',
9491018
r'^<asynchronous suspension>$',
950-
// Non-visible _onData frame.
1019+
r'^#2 awaitEveryAsyncStarThrowSync \(.+/utils.dart:104(:3)?\)$',
9511020
r'^<asynchronous suspension>$',
952-
r'^$',
9531021
];
954-
await doTestAwait(awaitEveryAsyncStarThrowSync, asyncStarThrowSyncExpected);
1022+
await doTestAwait(
1023+
awaitEveryAsyncStarThrowSync,
1024+
asyncStarThrowSyncExpected +
1025+
const <String>[
1026+
r'^#3 doTestAwait ',
1027+
r'^<asynchronous suspension>$',
1028+
r'^#4 doTestsLazy ',
1029+
r'^<asynchronous suspension>$',
1030+
r'^#5 main ',
1031+
r'^<asynchronous suspension>$',
1032+
r'^$',
1033+
]);
9551034
await doTestAwaitThen(
956-
awaitEveryAsyncStarThrowSync, asyncStarThrowSyncExpected);
1035+
awaitEveryAsyncStarThrowSync,
1036+
asyncStarThrowSyncExpected +
1037+
const <String>[
1038+
r'^#3 doTestAwaitThen.<anonymous closure> ',
1039+
r'^<asynchronous suspension>$',
1040+
r'^$',
1041+
]);
9571042
await doTestAwaitCatchError(
958-
awaitEveryAsyncStarThrowSync, asyncStarThrowSyncExpected);
1043+
awaitEveryAsyncStarThrowSync,
1044+
asyncStarThrowSyncExpected +
1045+
const <String>[
1046+
r'^$',
1047+
]);
9591048

9601049
final asyncStarThrowAsyncExpected = const <String>[
9611050
r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$',
9621051
r'^<asynchronous suspension>$',
9631052
r'^#1 asyncStarThrowAsync \(.*/utils.dart:126(:5)?\)$',
9641053
r'^<asynchronous suspension>$',
965-
// Non-visible _onData frame.
1054+
r'^#2 awaitEveryAsyncStarThrowAsync \(.+/utils.dart:117(:3)?\)$',
9661055
r'^<asynchronous suspension>$',
9671056
];
968-
await doTestAwait(awaitEveryAsyncStarThrowAsync, asyncStarThrowAsyncExpected);
1057+
await doTestAwait(
1058+
awaitEveryAsyncStarThrowAsync,
1059+
asyncStarThrowAsyncExpected +
1060+
const <String>[
1061+
r'^#3 doTestAwait ',
1062+
r'^<asynchronous suspension>$',
1063+
r'^#4 doTestsLazy ',
1064+
r'^<asynchronous suspension>$',
1065+
r'^#5 main ',
1066+
r'^<asynchronous suspension>$',
1067+
r'^$',
1068+
]);
9691069
await doTestAwaitThen(
970-
awaitEveryAsyncStarThrowAsync, asyncStarThrowAsyncExpected);
1070+
awaitEveryAsyncStarThrowAsync,
1071+
asyncStarThrowAsyncExpected +
1072+
const <String>[
1073+
r'^#3 doTestAwaitThen.<anonymous closure> ',
1074+
r'^<asynchronous suspension>$',
1075+
r'^$',
1076+
]);
9711077
await doTestAwaitCatchError(
972-
awaitEveryAsyncStarThrowAsync, asyncStarThrowAsyncExpected);
1078+
awaitEveryAsyncStarThrowAsync,
1079+
asyncStarThrowAsyncExpected +
1080+
const <String>[
1081+
r'^$',
1082+
]);
1083+
1084+
final listenAsyncStartExpected = const <String>[
1085+
r'^#0 throwAsync \(.*/utils.dart:21(:3)?\)$',
1086+
r'^<asynchronous suspension>$',
1087+
r'^#1 asyncStarThrowAsync \(.*/utils.dart:126(:5)?\)$',
1088+
r'^<asynchronous suspension>$',
1089+
r'^#2 listenAsyncStarThrowAsync.<anonymous closure> \(.+/utils.dart(:0)?\)$',
1090+
r'^<asynchronous suspension>$',
1091+
r'^$',
1092+
];
1093+
await doTestAwait(listenAsyncStarThrowAsync, listenAsyncStartExpected);
1094+
await doTestAwaitThen(listenAsyncStarThrowAsync, listenAsyncStartExpected);
1095+
await doTestAwaitCatchError(
1096+
listenAsyncStarThrowAsync, listenAsyncStartExpected);
9731097
}

‎runtime/vm/stack_trace.cc

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,22 +106,25 @@ class CallerClosureFinder {
106106
controller_(Object::Handle(zone)),
107107
state_(Object::Handle(zone)),
108108
var_data_(Object::Handle(zone)),
109+
callback_instance_(Object::Handle(zone)),
109110
future_impl_class(Class::Handle(zone)),
110111
async_await_completer_class(Class::Handle(zone)),
111112
future_listener_class(Class::Handle(zone)),
112113
async_start_stream_controller_class(Class::Handle(zone)),
113114
stream_controller_class(Class::Handle(zone)),
115+
async_stream_controller_class(Class::Handle(zone)),
114116
controller_subscription_class(Class::Handle(zone)),
115117
buffering_stream_subscription_class(Class::Handle(zone)),
116-
async_stream_controller_class(Class::Handle(zone)),
118+
stream_iterator_class(Class::Handle(zone)),
117119
completer_is_sync_field(Field::Handle(zone)),
118120
completer_future_field(Field::Handle(zone)),
119121
future_result_or_listeners_field(Field::Handle(zone)),
120122
callback_field(Field::Handle(zone)),
121123
controller_controller_field(Field::Handle(zone)),
122124
var_data_field(Field::Handle(zone)),
123125
state_field(Field::Handle(zone)),
124-
on_data_field(Field::Handle(zone)) {
126+
on_data_field(Field::Handle(zone)),
127+
state_data_field(Field::Handle(zone)) {
125128
const auto& async_lib = Library::Handle(zone, Library::AsyncLibrary());
126129
// Look up classes:
127130
// - async:
@@ -150,6 +153,9 @@ class CallerClosureFinder {
150153
buffering_stream_subscription_class = async_lib.LookupClassAllowPrivate(
151154
Symbols::_BufferingStreamSubscription());
152155
ASSERT(!buffering_stream_subscription_class.IsNull());
156+
stream_iterator_class =
157+
async_lib.LookupClassAllowPrivate(Symbols::_StreamIterator());
158+
ASSERT(!stream_iterator_class.IsNull());
153159

154160
// Look up fields:
155161
// - async:
@@ -180,15 +186,12 @@ class CallerClosureFinder {
180186
on_data_field = buffering_stream_subscription_class.LookupFieldAllowPrivate(
181187
Symbols::_onData());
182188
ASSERT(!on_data_field.IsNull());
189+
state_data_field =
190+
stream_iterator_class.LookupFieldAllowPrivate(Symbols::_stateData());
191+
ASSERT(!state_data_field.IsNull());
183192
}
184193

185-
RawClosure* FindCallerInAsyncClosure(const Context& receiver_context) {
186-
context_entry_ = receiver_context.At(Context::kAsyncCompleterIndex);
187-
ASSERT(context_entry_.IsInstance());
188-
ASSERT(context_entry_.GetClassId() == async_await_completer_class.id());
189-
190-
const Instance& completer = Instance::Cast(context_entry_);
191-
future_ = completer.GetField(completer_future_field);
194+
RawClosure* GetCallerInFutureImpl(const Object& future_) {
192195
ASSERT(!future_.IsNull());
193196
ASSERT(future_.GetClassId() == future_impl_class.id());
194197

@@ -208,6 +211,16 @@ class CallerClosureFinder {
208211
return Closure::Cast(callback_).raw();
209212
}
210213

214+
RawClosure* FindCallerInAsyncClosure(const Context& receiver_context) {
215+
context_entry_ = receiver_context.At(Context::kAsyncCompleterIndex);
216+
ASSERT(context_entry_.IsInstance());
217+
ASSERT(context_entry_.GetClassId() == async_await_completer_class.id());
218+
219+
const Instance& completer = Instance::Cast(context_entry_);
220+
future_ = completer.GetField(completer_future_field);
221+
return GetCallerInFutureImpl(future_);
222+
}
223+
211224
RawClosure* FindCallerInAsyncGenClosure(const Context& receiver_context) {
212225
context_entry_ = receiver_context.At(Context::kControllerIndex);
213226
ASSERT(context_entry_.IsInstance());
@@ -225,13 +238,37 @@ class CallerClosureFinder {
225238
return Closure::null();
226239
}
227240

241+
// _StreamController._varData
228242
var_data_ = Instance::Cast(controller_).GetField(var_data_field);
229243
ASSERT(var_data_.GetClassId() == controller_subscription_class.id());
230244

245+
// _ControllerSubscription<T>/_BufferingStreamSubscription.<T>_onData
231246
callback_ = Instance::Cast(var_data_).GetField(on_data_field);
232247
ASSERT(callback_.IsClosure());
233248

234-
return Closure::Cast(callback_).raw();
249+
// If this is not the "_StreamIterator._onData" tear-off, we return the
250+
// callback we found.
251+
receiver_function_ = Closure::Cast(callback_).function();
252+
if (!receiver_function_.IsImplicitInstanceClosureFunction() ||
253+
receiver_function_.Owner() != stream_iterator_class.raw()) {
254+
return Closure::Cast(callback_).raw();
255+
}
256+
257+
// All implicit closure functions (tear-offs) have the "this" receiver
258+
// captured.
259+
receiver_context_ = Closure::Cast(callback_).context();
260+
ASSERT(receiver_context_.num_variables() == 1);
261+
callback_instance_ = receiver_context_.At(0);
262+
ASSERT(callback_instance_.IsInstance());
263+
264+
// If the async* stream is await-for'd:
265+
if (callback_instance_.GetClassId() == stream_iterator_class.id()) {
266+
// _StreamIterator._stateData
267+
future_ = Instance::Cast(callback_instance_).GetField(state_data_field);
268+
return GetCallerInFutureImpl(future_);
269+
}
270+
271+
UNREACHABLE(); // If no onData is found we have a bug.
235272
}
236273

237274
RawClosure* FindCaller(const Closure& receiver_closure) {
@@ -284,15 +321,17 @@ class CallerClosureFinder {
284321
Object& controller_;
285322
Object& state_;
286323
Object& var_data_;
324+
Object& callback_instance_;
287325

288326
Class& future_impl_class;
289327
Class& async_await_completer_class;
290328
Class& future_listener_class;
291329
Class& async_start_stream_controller_class;
292330
Class& stream_controller_class;
331+
Class& async_stream_controller_class;
293332
Class& controller_subscription_class;
294333
Class& buffering_stream_subscription_class;
295-
Class& async_stream_controller_class;
334+
Class& stream_iterator_class;
296335

297336
Field& completer_is_sync_field;
298337
Field& completer_future_field;
@@ -302,6 +341,7 @@ class CallerClosureFinder {
302341
Field& var_data_field;
303342
Field& state_field;
304343
Field& on_data_field;
344+
Field& state_data_field;
305345
};
306346

307347
void StackTraceUtils::CollectFramesLazy(
@@ -351,7 +391,7 @@ void StackTraceUtils::CollectFramesLazy(
351391
if (frame->is_interpreted()) {
352392
code_array.Add(bytecode);
353393
const intptr_t pc_offset = frame->pc() - bytecode.PayloadStart();
354-
ASSERT(pc_offset >= 0 && pc_offset < bytecode.Size());
394+
ASSERT(pc_offset >= 0 && pc_offset <= bytecode.Size());
355395
offset = Smi::New(pc_offset);
356396
} else {
357397
code = frame->LookupDartCode();

0 commit comments

Comments
 (0)