Skip to content

Commit e505ab1

Browse files
alexmarkovCommit Bot
alexmarkov
authored and
Commit Bot
committed
[vm] Fix stack trace collection while registering async callbacks in the Zone
During 'await', when registering 'then' and 'error' callbacks in a custom Zone, the stack trace should be collected synchronously as there are no awaiters yet, but the suspended function and its callers are still on the stack. This change corrects the new implementation of async to collect these stack traces synchronously. Class StackZoneSpecification from package:stack_trace relies on the stack traces during callback registration in order to provide more detailed chain of async stack traces via Chain.capture. Without this change package:stack_trace captured truncated Chains. Issue: #48378 Issue: flutter/flutter#105340 TEST=vm/dart/causal_stacks/zone_callback_stack_traces_test Change-Id: Iee348aa5c9abb4126f6c5d8215a6dc2cee57e124 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/247620 Reviewed-by: Slava Egorov <[email protected]> Commit-Queue: Alexander Markov <[email protected]>
1 parent 3e9290d commit e505ab1

File tree

7 files changed

+207
-4
lines changed

7 files changed

+207
-4
lines changed

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

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
// Copyright (c) 2022, 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+
import 'dart:async';
6+
7+
import 'package:expect/expect.dart';
8+
9+
const String scriptName = 'zone_callback_stack_traces_test.dart';
10+
11+
Future<void> foo() async {}
12+
13+
Future<void> bar() async {
14+
await foo();
15+
}
16+
17+
Future<void> runTest() {
18+
final Zone testZone = Zone.current.fork(
19+
specification: ZoneSpecification(
20+
registerUnaryCallback: _registerUnaryCallback,
21+
registerBinaryCallback: _registerBinaryCallback));
22+
return testZone.run(bar);
23+
}
24+
25+
StackTrace? registerUnaryCallbackStackTrace;
26+
StackTrace? registerBinaryCallbackStackTrace;
27+
28+
ZoneUnaryCallback<R, T> _registerUnaryCallback<R, T>(
29+
Zone self, ZoneDelegate parent, Zone zone, R Function(T) f) {
30+
final stackTrace = StackTrace.current;
31+
print('registerUnaryCallback got stack trace:');
32+
print(stackTrace);
33+
if (stackTrace.toString().contains('bar')) {
34+
Expect.isNull(registerUnaryCallbackStackTrace);
35+
registerUnaryCallbackStackTrace = stackTrace;
36+
}
37+
return parent.registerUnaryCallback(zone, f);
38+
}
39+
40+
ZoneBinaryCallback<R, T1, T2> _registerBinaryCallback<R, T1, T2>(
41+
Zone self, ZoneDelegate parent, Zone zone, R Function(T1, T2) f) {
42+
final stackTrace = StackTrace.current;
43+
print('registerBinaryCallback got stack trace:');
44+
print(stackTrace);
45+
if (stackTrace.toString().contains('bar')) {
46+
Expect.isNull(registerBinaryCallbackStackTrace);
47+
registerBinaryCallbackStackTrace = stackTrace;
48+
}
49+
return parent.registerBinaryCallback(zone, f);
50+
}
51+
52+
void verifyStackTrace(List<String> expected, StackTrace stackTrace) {
53+
final List<String> actual = stackTrace
54+
.toString()
55+
.split('\n')
56+
.where((entry) => entry.contains(scriptName))
57+
.toList();
58+
print('Expected:\n${expected.join('\n')}');
59+
print('Actual:\n${actual.join('\n')}');
60+
Expect.equals(expected.length, actual.length);
61+
for (int i = 0; i < expected.length; ++i) {
62+
if (!RegExp(expected[i]).hasMatch(actual[i])) {
63+
Expect.fail("Stack trace entry $i doesn't match:\n"
64+
" expected: ${expected[i]}\n actual: ${actual[i]}");
65+
}
66+
}
67+
}
68+
69+
main() async {
70+
await runTest();
71+
verifyStackTrace([
72+
r'^#\d+ _registerUnaryCallback \(.*zone_callback_stack_traces_test.dart:30(:33)?\)$',
73+
r'^#\d+ bar \(.*zone_callback_stack_traces_test.dart:14(:9)?\)$',
74+
r'^#\d+ runTest \(.*zone_callback_stack_traces_test.dart:22(:19)?\)$',
75+
r'^#\d+ main \(.*zone_callback_stack_traces_test.dart:70(:9)?\)$',
76+
], registerUnaryCallbackStackTrace!);
77+
78+
verifyStackTrace([
79+
r'^#\d+ _registerBinaryCallback \(.*zone_callback_stack_traces_test.dart:42(:33)?\)$',
80+
r'^#\d+ bar \(.*zone_callback_stack_traces_test.dart:14(:9)?\)$',
81+
r'^#\d+ runTest \(.*zone_callback_stack_traces_test.dart:22(:19)?\)$',
82+
r'^#\d+ main \(.*zone_callback_stack_traces_test.dart:70(:9)?\)$',
83+
], registerBinaryCallbackStackTrace!);
84+
}

‎runtime/tests/vm/dart_2/causal_stacks/zone_callback_stack_traces_test.dart‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright (c) 2022, 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+
// @dart = 2.9
6+
7+
import 'dart:async';
8+
9+
import 'package:expect/expect.dart';
10+
11+
const String scriptName = 'zone_callback_stack_traces_test.dart';
12+
13+
Future<void> foo() async {}
14+
15+
Future<void> bar() async {
16+
await foo();
17+
}
18+
19+
Future<void> runTest() {
20+
final Zone testZone = Zone.current.fork(
21+
specification: ZoneSpecification(
22+
registerUnaryCallback: _registerUnaryCallback,
23+
registerBinaryCallback: _registerBinaryCallback));
24+
return testZone.run(bar);
25+
}
26+
27+
StackTrace registerUnaryCallbackStackTrace;
28+
StackTrace registerBinaryCallbackStackTrace;
29+
30+
ZoneUnaryCallback<R, T> _registerUnaryCallback<R, T>(
31+
Zone self, ZoneDelegate parent, Zone zone, R Function(T) f) {
32+
final stackTrace = StackTrace.current;
33+
print('registerUnaryCallback got stack trace:');
34+
print(stackTrace);
35+
if (stackTrace.toString().contains('bar')) {
36+
Expect.isNull(registerUnaryCallbackStackTrace);
37+
registerUnaryCallbackStackTrace = stackTrace;
38+
}
39+
return parent.registerUnaryCallback(zone, f);
40+
}
41+
42+
ZoneBinaryCallback<R, T1, T2> _registerBinaryCallback<R, T1, T2>(
43+
Zone self, ZoneDelegate parent, Zone zone, R Function(T1, T2) f) {
44+
final stackTrace = StackTrace.current;
45+
print('registerBinaryCallback got stack trace:');
46+
print(stackTrace);
47+
if (stackTrace.toString().contains('bar')) {
48+
Expect.isNull(registerBinaryCallbackStackTrace);
49+
registerBinaryCallbackStackTrace = stackTrace;
50+
}
51+
return parent.registerBinaryCallback(zone, f);
52+
}
53+
54+
void verifyStackTrace(List<String> expected, StackTrace stackTrace) {
55+
final List<String> actual = stackTrace
56+
.toString()
57+
.split('\n')
58+
.where((entry) => entry.contains(scriptName))
59+
.toList();
60+
print('Expected:\n${expected.join('\n')}');
61+
print('Actual:\n${actual.join('\n')}');
62+
Expect.equals(expected.length, actual.length);
63+
for (int i = 0; i < expected.length; ++i) {
64+
if (!RegExp(expected[i]).hasMatch(actual[i])) {
65+
Expect.fail("Stack trace entry $i doesn't match:\n"
66+
" expected: ${expected[i]}\n actual: ${actual[i]}");
67+
}
68+
}
69+
}
70+
71+
main() async {
72+
await runTest();
73+
verifyStackTrace([
74+
r'^#\d+ _registerUnaryCallback \(.*zone_callback_stack_traces_test.dart:32(:33)?\)$',
75+
r'^#\d+ bar \(.*zone_callback_stack_traces_test.dart:16(:9)?\)$',
76+
r'^#\d+ runTest \(.*zone_callback_stack_traces_test.dart:24(:19)?\)$',
77+
r'^#\d+ main \(.*zone_callback_stack_traces_test.dart:72(:9)?\)$',
78+
], registerUnaryCallbackStackTrace);
79+
80+
verifyStackTrace([
81+
r'^#\d+ _registerBinaryCallback \(.*zone_callback_stack_traces_test.dart:44(:33)?\)$',
82+
r'^#\d+ bar \(.*zone_callback_stack_traces_test.dart:16(:9)?\)$',
83+
r'^#\d+ runTest \(.*zone_callback_stack_traces_test.dart:24(:19)?\)$',
84+
r'^#\d+ main \(.*zone_callback_stack_traces_test.dart:72(:9)?\)$',
85+
], registerBinaryCallbackStackTrace);
86+
}

‎runtime/tests/vm/vm.status‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,10 +431,12 @@ dart/causal_stacks/async_throws_stack_lazy_test: SkipByDesign # Asserts exact st
431431
dart/causal_stacks/async_throws_stack_no_causal_test: SkipByDesign # Asserts exact stacktrace output.
432432
dart/causal_stacks/flutter_regress_100441_test: SkipByDesign # Asserts exact stacktrace output.
433433
dart/causal_stacks/sync_async_start_pkg_test_test: SkipByDesign # Asserts exact stacktrace output.
434+
dart/causal_stacks/zone_callback_stack_traces_test: SkipByDesign # Asserts exact stacktrace output.
434435
dart_2/causal_stacks/async_throws_stack_lazy_test: SkipByDesign # Asserts exact stacktrace output.
435436
dart_2/causal_stacks/async_throws_stack_no_causal_test: SkipByDesign # Asserts exact stacktrace output.
436437
dart_2/causal_stacks/flutter_regress_100441_test: SkipByDesign # Asserts exact stacktrace output.
437438
dart_2/causal_stacks/sync_async_start_pkg_test_test: SkipByDesign # Asserts exact stacktrace output.
439+
dart_2/causal_stacks/zone_callback_stack_traces_test: SkipByDesign # Asserts exact stacktrace output.
438440

439441
[ $compiler == dart2analyzer || $compiler == dart2js ]
440442
dart/data_uri*test: Skip # Data uri's not supported by dart2js or the analyzer.

‎runtime/vm/debugger.cc‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -768,7 +768,8 @@ ObjectPtr ActivationFrame::GetAsyncAwaiter(
768768
if (function_.IsCompactAsyncFunction() ||
769769
function_.IsCompactAsyncStarFunction()) {
770770
const auto& suspend_state = Object::Handle(GetSuspendStateVar());
771-
if (suspend_state.IsSuspendState()) {
771+
if (caller_closure_finder->WasPreviouslySuspended(function_,
772+
suspend_state)) {
772773
return caller_closure_finder->FindCallerFromSuspendState(
773774
SuspendState::Cast(suspend_state));
774775
}
@@ -846,7 +847,8 @@ bool ActivationFrame::HandlesException(const Instance& exc_obj) {
846847
} else if ((fp() != 0) && function().IsCompactAsyncFunction()) {
847848
CallerClosureFinder caller_closure_finder(Thread::Current()->zone());
848849
auto& suspend_state = Object::Handle(GetSuspendStateVar());
849-
if (!suspend_state.IsSuspendState()) {
850+
if (!caller_closure_finder.WasPreviouslySuspended(function(),
851+
suspend_state)) {
850852
return false;
851853
}
852854
Object& futureOrListener =
@@ -2117,7 +2119,8 @@ DebuggerStackTrace* DebuggerStackTrace::CollectAwaiterReturn() {
21172119
// Grab the awaiter.
21182120
async_activation ^= activation->GetAsyncAwaiter(&caller_closure_finder);
21192121
// Bail if we've reach the end of sync execution stack.
2120-
if (Object::Handle(activation->GetSuspendStateVar()).IsSuspendState()) {
2122+
if (caller_closure_finder.WasPreviouslySuspended(
2123+
function, Object::Handle(activation->GetSuspendStateVar()))) {
21212124
break;
21222125
}
21232126
} else {

‎runtime/vm/object.h‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11869,6 +11869,9 @@ class SuspendState : public Instance {
1186911869
InstancePtr function_data() const {
1187011870
return untag()->function_data();
1187111871
}
11872+
ClosurePtr error_callback() const {
11873+
return untag()->error_callback();
11874+
}
1187211875

1187311876
// Returns Code object corresponding to the suspended function.
1187411877
CodePtr GetCodeObject() const;

‎runtime/vm/stack_trace.cc‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,25 @@ bool CallerClosureFinder::IsRunningAsync(const Closure& receiver_closure) {
445445
return Bool::Cast(is_sync).value();
446446
}
447447

448+
bool CallerClosureFinder::WasPreviouslySuspended(
449+
const Function& function,
450+
const Object& suspend_state_var) {
451+
if (!suspend_state_var.IsSuspendState()) {
452+
return false;
453+
}
454+
if (function.IsCompactAsyncFunction()) {
455+
// Error callback is set after both 'then' and 'error' callbacks are
456+
// registered with the Zone. Callback registration may query
457+
// stack trace and should still collect the synchronous stack trace.
458+
return SuspendState::Cast(suspend_state_var).error_callback() !=
459+
Object::null();
460+
} else if (function.IsCompactAsyncStarFunction()) {
461+
return true;
462+
} else {
463+
UNREACHABLE();
464+
}
465+
}
466+
448467
ClosurePtr StackTraceUtils::FindClosureInFrame(ObjectPtr* last_object_in_caller,
449468
const Function& function) {
450469
NoSafepointScope nsp;
@@ -488,7 +507,8 @@ ClosurePtr StackTraceUtils::ClosureFromFrameFunction(
488507
zone, *reinterpret_cast<ObjectPtr*>(LocalVarAddress(
489508
frame->fp(), runtime_frame_layout.FrameSlotForVariableIndex(
490509
SuspendState::kSuspendStateVarIndex))));
491-
if (suspend_state.IsSuspendState()) {
510+
if (caller_closure_finder->WasPreviouslySuspended(function,
511+
suspend_state)) {
492512
*is_async = true;
493513
return caller_closure_finder->FindCallerFromSuspendState(
494514
SuspendState::Cast(suspend_state));

‎runtime/vm/stack_trace.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,11 @@ class CallerClosureFinder {
7272

7373
static bool IsRunningAsync(const Closure& receiver_closure);
7474

75+
// Tests if given [function] with given value of :suspend_state variable
76+
// was suspended at least once and running asynchronously.
77+
static bool WasPreviouslySuspended(const Function& function,
78+
const Object& suspend_state_var);
79+
7580
private:
7681
ClosurePtr FindCallerInternal(const Closure& receiver_closure);
7782
ClosurePtr GetCallerInFutureListenerInternal(const Object& future_listener);

0 commit comments

Comments
 (0)