Skip to content

Commit f4d9309

Browse files
sjindel-googlecommit-bot@chromium.org
sjindel-google
authored and
commit-bot@chromium.org
committed
[vm/async] Don't add an <asynchronous suspension> marker along sync-async calls.
This fixes `package:stack_trace` in the sync-start case of async/async* calls. Change-Id: I5c41a35283439f533ab19c9fd1cc542931de4b43 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124560 Commit-Queue: Samir Jindel <sjindel@google.com> Reviewed-by: Martin Kustermann <kustermann@google.com>
1 parent 4366afc commit f4d9309

15 files changed

+171
-78
lines changed

‎runtime/lib/stacktrace.cc‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ static RawStackTrace* CurrentSyncStackTrace(Thread* thread,
2323

2424
// Determine how big the stack trace is.
2525
const intptr_t stack_trace_length =
26-
StackTraceUtils::CountFrames(thread, skip_frames, null_function);
26+
StackTraceUtils::CountFrames(thread, skip_frames, null_function, nullptr);
2727

2828
// Allocate once.
2929
const Array& code_array =
@@ -64,8 +64,9 @@ static RawStackTrace* CurrentStackTrace(
6464

6565
// Determine the size of the stack trace.
6666
const intptr_t extra_frames = for_async_function ? 1 : 0;
67-
const intptr_t synchronous_stack_trace_length =
68-
StackTraceUtils::CountFrames(thread, skip_frames, async_function);
67+
bool sync_async_end = false;
68+
const intptr_t synchronous_stack_trace_length = StackTraceUtils::CountFrames(
69+
thread, skip_frames, async_function, &sync_async_end);
6970

7071
const intptr_t capacity = synchronous_stack_trace_length +
7172
extra_frames; // For the asynchronous gap.
@@ -94,7 +95,11 @@ static RawStackTrace* CurrentStackTrace(
9495

9596
ASSERT(write_cursor == capacity);
9697

97-
return StackTrace::New(code_array, pc_offset_array, async_stack_trace);
98+
const StackTrace& result = StackTrace::Handle(
99+
zone, StackTrace::New(code_array, pc_offset_array, async_stack_trace,
100+
sync_async_end));
101+
102+
return result.raw();
98103
}
99104

100105
RawStackTrace* GetStackTraceForException() {

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,16 @@ import 'service_test_common.dart';
1111
import 'test_helper.dart';
1212

1313
const LINE_C = 19;
14-
const LINE_A = 24;
15-
const LINE_B = 30;
14+
const LINE_A = 25;
15+
const LINE_B = 31;
1616

1717
foobar() {
1818
debugger();
1919
print('foobar'); // LINE_C.
2020
}
2121

2222
helper() async {
23+
await 0; // force async gap
2324
debugger();
2425
print('helper'); // LINE_A.
2526
foobar();
@@ -64,10 +65,10 @@ var tests = <IsolateTest>[
6465
expect(asyncStack[3].toString(), contains('testMain'));
6566
// Line 19.
6667
expect(await asyncStack[0].location.toUserString(), contains('.dart:19'));
67-
// Line 25.
68-
expect(await asyncStack[1].location.toUserString(), contains('.dart:25'));
69-
// Line 30.
70-
expect(await asyncStack[3].location.toUserString(), contains('.dart:30'));
68+
// Line 26.
69+
expect(await asyncStack[1].location.toUserString(), contains('.dart:26'));
70+
// Line 31.
71+
expect(await asyncStack[3].location.toUserString(), contains('.dart:31'));
7172
},
7273
];
7374

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,18 +10,20 @@ import 'package:unittest/unittest.dart';
1010
import 'service_test_common.dart';
1111
import 'test_helper.dart';
1212

13-
const LINE_A = 26;
14-
const LINE_B = 19;
15-
const LINE_C = 21;
13+
const LINE_A = 28;
14+
const LINE_B = 20;
15+
const LINE_C = 22;
1616

1717
foobar() async* {
18+
await 0; // force async gap
1819
debugger();
1920
yield 1; // LINE_B.
2021
debugger();
2122
yield 2; // LINE_C.
2223
}
2324

2425
helper() async {
26+
await 0; // force async gap
2527
debugger();
2628
print('helper'); // LINE_A.
2729
await for (var i in foobar()) {
@@ -76,12 +78,13 @@ var tests = <IsolateTest>[
7678
expect(asyncStack[2].toString(), contains('helper'));
7779
expect(asyncStack[3].kind, equals(M.FrameKind.asyncSuspensionMarker));
7880
expect(asyncStack[4].toString(), contains('testMain'));
79-
// Line 21.
80-
expect(await asyncStack[0].location.toUserString(), contains('.dart:21'));
81-
// Line 27.
82-
expect(await asyncStack[2].location.toUserString(), contains('.dart:27'));
83-
// Line 30.
84-
expect(await asyncStack[4].location.toUserString(), contains('.dart:33'));
81+
// Line 22.
82+
expect(
83+
await asyncStack[0].location.toUserString(), contains('.dart:$LINE_C'));
84+
// Line 29.
85+
expect(await asyncStack[2].location.toUserString(), contains('.dart:29'));
86+
// Line 35.
87+
expect(await asyncStack[4].location.toUserString(), contains('.dart:35'));
8588
},
8689
];
8790

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import 'package:unittest/unittest.dart';
1010
import 'test_helper.dart';
1111
import 'service_test_common.dart';
1212

13-
const LINE_A = 34;
13+
const LINE_A = 35;
1414

1515
class Foo {}
1616

@@ -20,6 +20,7 @@ doThrow() {
2020
}
2121

2222
asyncThrower() async {
23+
await 0; // force async gap
2324
doThrow();
2425
}
2526

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import 'package:unittest/unittest.dart';
99
import 'test_helper.dart';
1010
import 'service_test_common.dart';
1111

12-
const LINE_A = 34;
12+
const LINE_A = 35;
1313

1414
class Foo {}
1515

@@ -19,6 +19,7 @@ doThrow() {
1919
}
2020

2121
asyncThrower() async {
22+
await 0; // force async gap
2223
doThrow();
2324
}
2425

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

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,8 @@ Future<void> noYields3() async {
124124
Map<int, String> noYieldsMapCausal = {
125125
0: '#0 throwSync ',
126126
1: '#1 noYields3 ',
127-
2: '<asynchronous suspension>',
128-
3: '#2 noYields2 ',
129-
4: '<asynchronous suspension>',
130-
5: '#3 noYields ',
131-
4: '<asynchronous suspension>',
127+
2: '#2 noYields2 ',
128+
3: '#3 noYields ',
132129
// Callers, like doTest and main ..
133130
};
134131

@@ -168,11 +165,9 @@ Map<int, String> mixedYieldsMapCausal = {
168165
0: '#0 throwAsync ',
169166
1: '<asynchronous suspension>',
170167
2: '#1 mixedYields3 ',
171-
3: '<asynchronous suspension>',
172-
4: '#2 mixedYields2 ',
173-
5: '<asynchronous suspension>',
174-
6: '#3 mixedYields ',
175-
7: '<asynchronous suspension>',
168+
3: '#2 mixedYields2 ',
169+
4: '<asynchronous suspension>',
170+
5: '#3 mixedYields ',
176171
// Callers, like doTest and main ..
177172
};
178173

@@ -207,7 +202,6 @@ Map<int, String> syncSuffixMapCausal = {
207202
3: '#2 syncSuffix2 ',
208203
4: '<asynchronous suspension>',
209204
5: '#3 syncSuffix ',
210-
6: '<asynchronous suspension>',
211205
// Callers, like doTest and main ..
212206
};
213207

‎runtime/tests/vm/dart/sync_async_start_pkg_test_test.dart‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright (c) 2019, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
//
5+
// This test ensures that "pkg:stack_trace" (used by "pkg:test") doesn't break
6+
// when causal async stacks are enabled by dropping frames below a synchronous
7+
// start to an async function.
8+
9+
import "package:test/test.dart";
10+
import "package:stack_trace/src/stack_zone_specification.dart";
11+
12+
import 'dart:async';
13+
14+
void main() {
15+
test("Stacktrace includes sync-starts.", () async {
16+
final st = await firstMethod();
17+
expect("$st", allOf([contains("firstMethod"), contains("secondMethod")]));
18+
});
19+
}
20+
21+
Future<StackTrace> firstMethod() async {
22+
return await secondMethod();
23+
}
24+
25+
Future<StackTrace> secondMethod() async {
26+
return StackTrace.current;
27+
}

‎runtime/vm/debugger.cc‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2266,8 +2266,9 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() {
22662266
return NULL;
22672267
}
22682268

2269+
bool sync_async_end = false;
22692270
intptr_t synchronous_stack_trace_length =
2270-
StackTraceUtils::CountFrames(thread, 0, async_function);
2271+
StackTraceUtils::CountFrames(thread, 0, async_function, &sync_async_end);
22712272

22722273
// Append the top frames from the synchronous stack trace, up until the active
22732274
// asynchronous function. We truncate the remainder of the synchronous
@@ -2304,8 +2305,11 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() {
23042305
// Now we append the asynchronous causal stack trace. These are not active
23052306
// frames but a historical record of how this asynchronous function was
23062307
// activated.
2308+
2309+
intptr_t frame_skip =
2310+
sync_async_end ? StackTrace::kSyncAsyncCroppedFrames : 0;
23072311
while (!async_stack_trace.IsNull()) {
2308-
for (intptr_t i = 0; i < async_stack_trace.Length(); i++) {
2312+
for (intptr_t i = frame_skip; i < async_stack_trace.Length(); i++) {
23092313
code_obj = async_stack_trace.CodeAtFrame(i);
23102314
if (code_obj.IsNull()) {
23112315
break;
@@ -2340,22 +2344,15 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() {
23402344
}
23412345
}
23422346
// Follow the link.
2347+
frame_skip = async_stack_trace.skip_sync_start_in_parent_stack()
2348+
? StackTrace::kSyncAsyncCroppedFrames
2349+
: 0;
23432350
async_stack_trace = async_stack_trace.async_link();
23442351
}
23452352

23462353
return stack_trace;
23472354
}
23482355

2349-
#if !defined(DART_PRECOMPILED_RUNTIME)
2350-
static bool CheckAndSkipAsync(int skip_sync_async_frames_count,
2351-
const String& function_name) {
2352-
return (skip_sync_async_frames_count == 2 &&
2353-
function_name.Equals(Symbols::_ClosureCall())) ||
2354-
(skip_sync_async_frames_count == 1 &&
2355-
function_name.Equals(Symbols::_AsyncAwaitCompleterStart()));
2356-
}
2357-
#endif
2358-
23592356
DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() {
23602357
#if defined(DART_PRECOMPILED_RUNTIME)
23612358
// Causal async stacks are not supported in the AOT runtime.
@@ -2407,7 +2404,8 @@ DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() {
24072404

24082405
if (skip_sync_async_frames_count > 0) {
24092406
function_name = function.QualifiedScrubbedName();
2410-
if (CheckAndSkipAsync(skip_sync_async_frames_count, function_name)) {
2407+
if (StackTraceUtils::CheckAndSkipAsync(skip_sync_async_frames_count,
2408+
function_name)) {
24112409
skip_sync_async_frames_count--;
24122410
} else {
24132411
// Unexpected function in synchronous call of async function.
@@ -2457,8 +2455,8 @@ DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() {
24572455

24582456
if (skip_sync_async_frames_count > 0) {
24592457
function_name ^= function.QualifiedScrubbedName();
2460-
if (CheckAndSkipAsync(skip_sync_async_frames_count,
2461-
function_name)) {
2458+
if (StackTraceUtils::CheckAndSkipAsync(
2459+
skip_sync_async_frames_count, function_name)) {
24622460
skip_sync_async_frames_count--;
24632461
} else {
24642462
// Unexpected function in sync async call
@@ -2512,8 +2510,8 @@ DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() {
25122510

25132511
if (skip_sync_async_frames_count > 0) {
25142512
function_name ^= function.QualifiedScrubbedName();
2515-
if (CheckAndSkipAsync(skip_sync_async_frames_count,
2516-
function_name)) {
2513+
if (StackTraceUtils::CheckAndSkipAsync(skip_sync_async_frames_count,
2514+
function_name)) {
25172515
skip_sync_async_frames_count--;
25182516
} else {
25192517
// Unexpected function in synchronous call of async function.

‎runtime/vm/object.cc‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21886,6 +21886,14 @@ RawFunction* Closure::GetInstantiatedSignature(Zone* zone) const {
2188621886
return sig_fun.raw();
2188721887
}
2188821888

21889+
bool StackTrace::skip_sync_start_in_parent_stack() const {
21890+
return raw_ptr()->skip_sync_start_in_parent_stack;
21891+
}
21892+
21893+
void StackTrace::set_skip_sync_start_in_parent_stack(bool value) const {
21894+
StoreNonPointer(&raw_ptr()->skip_sync_start_in_parent_stack, value);
21895+
}
21896+
2188921897
intptr_t StackTrace::Length() const {
2189021898
const Array& code_array = Array::Handle(raw_ptr()->code_array_);
2189121899
return code_array.Length();
@@ -21946,12 +21954,14 @@ RawStackTrace* StackTrace::New(const Array& code_array,
2194621954
result.set_code_array(code_array);
2194721955
result.set_pc_offset_array(pc_offset_array);
2194821956
result.set_expand_inlined(true); // default.
21957+
result.set_skip_sync_start_in_parent_stack(false);
2194921958
return result.raw();
2195021959
}
2195121960

2195221961
RawStackTrace* StackTrace::New(const Array& code_array,
2195321962
const Array& pc_offset_array,
2195421963
const StackTrace& async_link,
21964+
bool skip_sync_start_in_parent_stack,
2195521965
Heap::Space space) {
2195621966
StackTrace& result = StackTrace::Handle();
2195721967
{
@@ -21964,6 +21974,7 @@ RawStackTrace* StackTrace::New(const Array& code_array,
2196421974
result.set_code_array(code_array);
2196521975
result.set_pc_offset_array(pc_offset_array);
2196621976
result.set_expand_inlined(true); // default.
21977+
result.set_skip_sync_start_in_parent_stack(skip_sync_start_in_parent_stack);
2196721978
return result.raw();
2196821979
}
2196921980

@@ -22022,8 +22033,9 @@ const char* StackTrace::ToDartCString(const StackTrace& stack_trace_in) {
2202222033
// Iterate through the stack frames and create C string description
2202322034
// for each frame.
2202422035
intptr_t frame_index = 0;
22036+
uint32_t frame_skip = 0;
2202522037
do {
22026-
for (intptr_t i = 0; i < stack_trace.Length(); i++) {
22038+
for (intptr_t i = frame_skip; i < stack_trace.Length(); i++) {
2202722039
code_object = stack_trace.CodeAtFrame(i);
2202822040
if (code_object.IsNull()) {
2202922041
// Check for a null function, which indicates a gap in a StackOverflow
@@ -22083,6 +22095,9 @@ const char* StackTrace::ToDartCString(const StackTrace& stack_trace_in) {
2208322095
}
2208422096
}
2208522097
// Follow the link.
22098+
frame_skip = stack_trace.skip_sync_start_in_parent_stack()
22099+
? StackTrace::kSyncAsyncCroppedFrames
22100+
: 0;
2208622101
stack_trace = stack_trace.async_link();
2208722102
} while (!stack_trace.IsNull());
2208822103

@@ -22110,8 +22125,9 @@ const char* StackTrace::ToDwarfCString(const StackTrace& stack_trace_in) {
2211022125
buffer.Printf("pid: %" Pd ", tid: %" Pd ", name %s\n", OS::ProcessId(),
2211122126
OSThread::ThreadIdToIntPtr(thread->id()), thread->name());
2211222127
intptr_t frame_index = 0;
22128+
uint32_t frame_skip = 0;
2211322129
do {
22114-
for (intptr_t i = 0; i < stack_trace.Length(); i++) {
22130+
for (intptr_t i = frame_skip; i < stack_trace.Length(); i++) {
2211522131
code = stack_trace.CodeAtFrame(i);
2211622132
if (code.IsNull()) {
2211722133
// Check for a null function, which indicates a gap in a StackOverflow
@@ -22153,6 +22169,9 @@ const char* StackTrace::ToDwarfCString(const StackTrace& stack_trace_in) {
2215322169
}
2215422170
}
2215522171
// Follow the link.
22172+
frame_skip = stack_trace.skip_sync_start_in_parent_stack()
22173+
? StackTrace::kSyncAsyncCroppedFrames
22174+
: 0;
2215622175
stack_trace = stack_trace.async_link();
2215722176
} while (!stack_trace.IsNull());
2215822177

0 commit comments

Comments
 (0)