[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: https://github.com/dart-lang/sdk/issues/48378
Issue: https://github.com/flutter/flutter/issues/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 <vegorov@google.com>
Commit-Queue: Alexander Markov <alexmarkov@google.com>
diff --git a/runtime/tests/vm/dart/causal_stacks/zone_callback_stack_traces_test.dart b/runtime/tests/vm/dart/causal_stacks/zone_callback_stack_traces_test.dart
new file mode 100644
index 0000000..fa42d00
--- /dev/null
+++ b/runtime/tests/vm/dart/causal_stacks/zone_callback_stack_traces_test.dart
@@ -0,0 +1,84 @@
+// Copyright (c) 2022, 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.
+
+import 'dart:async';
+
+import 'package:expect/expect.dart';
+
+const String scriptName = 'zone_callback_stack_traces_test.dart';
+
+Future<void> foo() async {}
+
+Future<void> bar() async {
+ await foo();
+}
+
+Future<void> runTest() {
+ final Zone testZone = Zone.current.fork(
+ specification: ZoneSpecification(
+ registerUnaryCallback: _registerUnaryCallback,
+ registerBinaryCallback: _registerBinaryCallback));
+ return testZone.run(bar);
+}
+
+StackTrace? registerUnaryCallbackStackTrace;
+StackTrace? registerBinaryCallbackStackTrace;
+
+ZoneUnaryCallback<R, T> _registerUnaryCallback<R, T>(
+ Zone self, ZoneDelegate parent, Zone zone, R Function(T) f) {
+ final stackTrace = StackTrace.current;
+ print('registerUnaryCallback got stack trace:');
+ print(stackTrace);
+ if (stackTrace.toString().contains('bar')) {
+ Expect.isNull(registerUnaryCallbackStackTrace);
+ registerUnaryCallbackStackTrace = stackTrace;
+ }
+ return parent.registerUnaryCallback(zone, f);
+}
+
+ZoneBinaryCallback<R, T1, T2> _registerBinaryCallback<R, T1, T2>(
+ Zone self, ZoneDelegate parent, Zone zone, R Function(T1, T2) f) {
+ final stackTrace = StackTrace.current;
+ print('registerBinaryCallback got stack trace:');
+ print(stackTrace);
+ if (stackTrace.toString().contains('bar')) {
+ Expect.isNull(registerBinaryCallbackStackTrace);
+ registerBinaryCallbackStackTrace = stackTrace;
+ }
+ return parent.registerBinaryCallback(zone, f);
+}
+
+void verifyStackTrace(List<String> expected, StackTrace stackTrace) {
+ final List<String> actual = stackTrace
+ .toString()
+ .split('\n')
+ .where((entry) => entry.contains(scriptName))
+ .toList();
+ print('Expected:\n${expected.join('\n')}');
+ print('Actual:\n${actual.join('\n')}');
+ Expect.equals(expected.length, actual.length);
+ for (int i = 0; i < expected.length; ++i) {
+ if (!RegExp(expected[i]).hasMatch(actual[i])) {
+ Expect.fail("Stack trace entry $i doesn't match:\n"
+ " expected: ${expected[i]}\n actual: ${actual[i]}");
+ }
+ }
+}
+
+main() async {
+ await runTest();
+ verifyStackTrace([
+ r'^#\d+ _registerUnaryCallback \(.*zone_callback_stack_traces_test.dart:30(:33)?\)$',
+ r'^#\d+ bar \(.*zone_callback_stack_traces_test.dart:14(:9)?\)$',
+ r'^#\d+ runTest \(.*zone_callback_stack_traces_test.dart:22(:19)?\)$',
+ r'^#\d+ main \(.*zone_callback_stack_traces_test.dart:70(:9)?\)$',
+ ], registerUnaryCallbackStackTrace!);
+
+ verifyStackTrace([
+ r'^#\d+ _registerBinaryCallback \(.*zone_callback_stack_traces_test.dart:42(:33)?\)$',
+ r'^#\d+ bar \(.*zone_callback_stack_traces_test.dart:14(:9)?\)$',
+ r'^#\d+ runTest \(.*zone_callback_stack_traces_test.dart:22(:19)?\)$',
+ r'^#\d+ main \(.*zone_callback_stack_traces_test.dart:70(:9)?\)$',
+ ], registerBinaryCallbackStackTrace!);
+}
diff --git a/runtime/tests/vm/dart_2/causal_stacks/zone_callback_stack_traces_test.dart b/runtime/tests/vm/dart_2/causal_stacks/zone_callback_stack_traces_test.dart
new file mode 100644
index 0000000..02744ce
--- /dev/null
+++ b/runtime/tests/vm/dart_2/causal_stacks/zone_callback_stack_traces_test.dart
@@ -0,0 +1,86 @@
+// Copyright (c) 2022, 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.
+
+// @dart = 2.9
+
+import 'dart:async';
+
+import 'package:expect/expect.dart';
+
+const String scriptName = 'zone_callback_stack_traces_test.dart';
+
+Future<void> foo() async {}
+
+Future<void> bar() async {
+ await foo();
+}
+
+Future<void> runTest() {
+ final Zone testZone = Zone.current.fork(
+ specification: ZoneSpecification(
+ registerUnaryCallback: _registerUnaryCallback,
+ registerBinaryCallback: _registerBinaryCallback));
+ return testZone.run(bar);
+}
+
+StackTrace registerUnaryCallbackStackTrace;
+StackTrace registerBinaryCallbackStackTrace;
+
+ZoneUnaryCallback<R, T> _registerUnaryCallback<R, T>(
+ Zone self, ZoneDelegate parent, Zone zone, R Function(T) f) {
+ final stackTrace = StackTrace.current;
+ print('registerUnaryCallback got stack trace:');
+ print(stackTrace);
+ if (stackTrace.toString().contains('bar')) {
+ Expect.isNull(registerUnaryCallbackStackTrace);
+ registerUnaryCallbackStackTrace = stackTrace;
+ }
+ return parent.registerUnaryCallback(zone, f);
+}
+
+ZoneBinaryCallback<R, T1, T2> _registerBinaryCallback<R, T1, T2>(
+ Zone self, ZoneDelegate parent, Zone zone, R Function(T1, T2) f) {
+ final stackTrace = StackTrace.current;
+ print('registerBinaryCallback got stack trace:');
+ print(stackTrace);
+ if (stackTrace.toString().contains('bar')) {
+ Expect.isNull(registerBinaryCallbackStackTrace);
+ registerBinaryCallbackStackTrace = stackTrace;
+ }
+ return parent.registerBinaryCallback(zone, f);
+}
+
+void verifyStackTrace(List<String> expected, StackTrace stackTrace) {
+ final List<String> actual = stackTrace
+ .toString()
+ .split('\n')
+ .where((entry) => entry.contains(scriptName))
+ .toList();
+ print('Expected:\n${expected.join('\n')}');
+ print('Actual:\n${actual.join('\n')}');
+ Expect.equals(expected.length, actual.length);
+ for (int i = 0; i < expected.length; ++i) {
+ if (!RegExp(expected[i]).hasMatch(actual[i])) {
+ Expect.fail("Stack trace entry $i doesn't match:\n"
+ " expected: ${expected[i]}\n actual: ${actual[i]}");
+ }
+ }
+}
+
+main() async {
+ await runTest();
+ verifyStackTrace([
+ r'^#\d+ _registerUnaryCallback \(.*zone_callback_stack_traces_test.dart:32(:33)?\)$',
+ r'^#\d+ bar \(.*zone_callback_stack_traces_test.dart:16(:9)?\)$',
+ r'^#\d+ runTest \(.*zone_callback_stack_traces_test.dart:24(:19)?\)$',
+ r'^#\d+ main \(.*zone_callback_stack_traces_test.dart:72(:9)?\)$',
+ ], registerUnaryCallbackStackTrace);
+
+ verifyStackTrace([
+ r'^#\d+ _registerBinaryCallback \(.*zone_callback_stack_traces_test.dart:44(:33)?\)$',
+ r'^#\d+ bar \(.*zone_callback_stack_traces_test.dart:16(:9)?\)$',
+ r'^#\d+ runTest \(.*zone_callback_stack_traces_test.dart:24(:19)?\)$',
+ r'^#\d+ main \(.*zone_callback_stack_traces_test.dart:72(:9)?\)$',
+ ], registerBinaryCallbackStackTrace);
+}
diff --git a/runtime/tests/vm/vm.status b/runtime/tests/vm/vm.status
index 2d0f5db..fb489d4 100644
--- a/runtime/tests/vm/vm.status
+++ b/runtime/tests/vm/vm.status
@@ -431,10 +431,12 @@
dart/causal_stacks/async_throws_stack_no_causal_test: SkipByDesign # Asserts exact stacktrace output.
dart/causal_stacks/flutter_regress_100441_test: SkipByDesign # Asserts exact stacktrace output.
dart/causal_stacks/sync_async_start_pkg_test_test: SkipByDesign # Asserts exact stacktrace output.
+dart/causal_stacks/zone_callback_stack_traces_test: SkipByDesign # Asserts exact stacktrace output.
dart_2/causal_stacks/async_throws_stack_lazy_test: SkipByDesign # Asserts exact stacktrace output.
dart_2/causal_stacks/async_throws_stack_no_causal_test: SkipByDesign # Asserts exact stacktrace output.
dart_2/causal_stacks/flutter_regress_100441_test: SkipByDesign # Asserts exact stacktrace output.
dart_2/causal_stacks/sync_async_start_pkg_test_test: SkipByDesign # Asserts exact stacktrace output.
+dart_2/causal_stacks/zone_callback_stack_traces_test: SkipByDesign # Asserts exact stacktrace output.
[ $compiler == dart2analyzer || $compiler == dart2js ]
dart/data_uri*test: Skip # Data uri's not supported by dart2js or the analyzer.
diff --git a/runtime/vm/debugger.cc b/runtime/vm/debugger.cc
index 1a83328..31cc36f 100644
--- a/runtime/vm/debugger.cc
+++ b/runtime/vm/debugger.cc
@@ -768,7 +768,8 @@
if (function_.IsCompactAsyncFunction() ||
function_.IsCompactAsyncStarFunction()) {
const auto& suspend_state = Object::Handle(GetSuspendStateVar());
- if (suspend_state.IsSuspendState()) {
+ if (caller_closure_finder->WasPreviouslySuspended(function_,
+ suspend_state)) {
return caller_closure_finder->FindCallerFromSuspendState(
SuspendState::Cast(suspend_state));
}
@@ -846,7 +847,8 @@
} else if ((fp() != 0) && function().IsCompactAsyncFunction()) {
CallerClosureFinder caller_closure_finder(Thread::Current()->zone());
auto& suspend_state = Object::Handle(GetSuspendStateVar());
- if (!suspend_state.IsSuspendState()) {
+ if (!caller_closure_finder.WasPreviouslySuspended(function(),
+ suspend_state)) {
return false;
}
Object& futureOrListener =
@@ -2117,7 +2119,8 @@
// Grab the awaiter.
async_activation ^= activation->GetAsyncAwaiter(&caller_closure_finder);
// Bail if we've reach the end of sync execution stack.
- if (Object::Handle(activation->GetSuspendStateVar()).IsSuspendState()) {
+ if (caller_closure_finder.WasPreviouslySuspended(
+ function, Object::Handle(activation->GetSuspendStateVar()))) {
break;
}
} else {
diff --git a/runtime/vm/object.h b/runtime/vm/object.h
index 8b3c7a3..bdb1787 100644
--- a/runtime/vm/object.h
+++ b/runtime/vm/object.h
@@ -11869,6 +11869,9 @@
InstancePtr function_data() const {
return untag()->function_data();
}
+ ClosurePtr error_callback() const {
+ return untag()->error_callback();
+ }
// Returns Code object corresponding to the suspended function.
CodePtr GetCodeObject() const;
diff --git a/runtime/vm/stack_trace.cc b/runtime/vm/stack_trace.cc
index a5c7bdd..671f58b 100644
--- a/runtime/vm/stack_trace.cc
+++ b/runtime/vm/stack_trace.cc
@@ -445,6 +445,25 @@
return Bool::Cast(is_sync).value();
}
+bool CallerClosureFinder::WasPreviouslySuspended(
+ const Function& function,
+ const Object& suspend_state_var) {
+ if (!suspend_state_var.IsSuspendState()) {
+ return false;
+ }
+ if (function.IsCompactAsyncFunction()) {
+ // Error callback is set after both 'then' and 'error' callbacks are
+ // registered with the Zone. Callback registration may query
+ // stack trace and should still collect the synchronous stack trace.
+ return SuspendState::Cast(suspend_state_var).error_callback() !=
+ Object::null();
+ } else if (function.IsCompactAsyncStarFunction()) {
+ return true;
+ } else {
+ UNREACHABLE();
+ }
+}
+
ClosurePtr StackTraceUtils::FindClosureInFrame(ObjectPtr* last_object_in_caller,
const Function& function) {
NoSafepointScope nsp;
@@ -488,7 +507,8 @@
zone, *reinterpret_cast<ObjectPtr*>(LocalVarAddress(
frame->fp(), runtime_frame_layout.FrameSlotForVariableIndex(
SuspendState::kSuspendStateVarIndex))));
- if (suspend_state.IsSuspendState()) {
+ if (caller_closure_finder->WasPreviouslySuspended(function,
+ suspend_state)) {
*is_async = true;
return caller_closure_finder->FindCallerFromSuspendState(
SuspendState::Cast(suspend_state));
diff --git a/runtime/vm/stack_trace.h b/runtime/vm/stack_trace.h
index 2e33b71..37df069 100644
--- a/runtime/vm/stack_trace.h
+++ b/runtime/vm/stack_trace.h
@@ -72,6 +72,11 @@
static bool IsRunningAsync(const Closure& receiver_closure);
+ // Tests if given [function] with given value of :suspend_state variable
+ // was suspended at least once and running asynchronously.
+ static bool WasPreviouslySuspended(const Function& function,
+ const Object& suspend_state_var);
+
private:
ClosurePtr FindCallerInternal(const Closure& receiver_closure);
ClosurePtr GetCallerInFutureListenerInternal(const Object& future_listener);
