dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

Revert "[vm/async] Don't add an <asynchronous suspension> marker along sync-async calls."

Browse Source 
This reverts commit f4d930997b.

Reason for revert: Different tests are failing on different bots.

Original change's description:
> [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>

TBR=kustermann@google.com,sjindel@google.com

Change-Id: Ib3e1b303613f0393dac6b1b81e8d1ffb322052b8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/124800
Reviewed-by: Régis Crelier <regis@google.com>
Commit-Queue: Régis Crelier <regis@google.com>
This commit is contained in:
Régis Crelier
2019-11-11 19:03:54 +00:00
committed by commit-bot@chromium.org
parent 263ac017be
commit b5110a59a9
15 changed files with 78 additions and 171 deletions
Download Patch File Download Diff File Expand all files Collapse all files
runtime/lib/stacktrace.cc
+4 -9
View File
@@ -23,7 +23,7 @@ static RawStackTrace* CurrentSyncStackTrace(Thread* thread,
// Determine how big the stack trace is.
const intptr_t stack_trace_length =
StackTraceUtils::CountFrames(thread, skip_frames, null_function, nullptr);
StackTraceUtils::CountFrames(thread, skip_frames, null_function);
// Allocate once.
const Array& code_array =
@@ -64,9 +64,8 @@ static RawStackTrace* CurrentStackTrace(
// Determine the size of the stack trace.
const intptr_t extra_frames = for_async_function ? 1 : 0;
bool sync_async_end = false;
const intptr_t synchronous_stack_trace_length = StackTraceUtils::CountFrames(
thread, skip_frames, async_function, &sync_async_end);
const intptr_t synchronous_stack_trace_length =
StackTraceUtils::CountFrames(thread, skip_frames, async_function);
const intptr_t capacity = synchronous_stack_trace_length +
extra_frames; // For the asynchronous gap.
@@ -95,11 +94,7 @@ static RawStackTrace* CurrentStackTrace(
ASSERT(write_cursor == capacity);
const StackTrace& result = StackTrace::Handle(
zone, StackTrace::New(code_array, pc_offset_array, async_stack_trace,
sync_async_end));
return result.raw();
return StackTrace::New(code_array, pc_offset_array, async_stack_trace);
}
RawStackTrace* GetStackTraceForException() {
runtime/observatory/tests/service/causal_async_stack_contents_test.dart
+6 -7
View File
@@ -11,8 +11,8 @@ import 'service_test_common.dart';
import 'test_helper.dart';
const LINE_C = 19;
const LINE_A = 25;
const LINE_B = 31;
const LINE_A = 24;
const LINE_B = 30;
foobar() {
debugger();
@@ -20,7 +20,6 @@ foobar() {
}
helper() async {
await 0; // force async gap
debugger();
print('helper'); // LINE_A.
foobar();
@@ -65,10 +64,10 @@ var tests = <IsolateTest>[
expect(asyncStack[3].toString(), contains('testMain'));
// Line 19.
expect(await asyncStack[0].location.toUserString(), contains('.dart:19'));
// Line 26.
expect(await asyncStack[1].location.toUserString(), contains('.dart:26'));
// Line 31.
expect(await asyncStack[3].location.toUserString(), contains('.dart:31'));
// Line 25.
expect(await asyncStack[1].location.toUserString(), contains('.dart:25'));
// Line 30.
expect(await asyncStack[3].location.toUserString(), contains('.dart:30'));
},
];
runtime/observatory/tests/service/causal_async_star_stack_contents_test.dart
+9 -12
View File
@@ -10,12 +10,11 @@ import 'package:unittest/unittest.dart';
import 'service_test_common.dart';
import 'test_helper.dart';
const LINE_A = 28;
const LINE_B = 20;
const LINE_C = 22;
const LINE_A = 26;
const LINE_B = 19;
const LINE_C = 21;
foobar() async* {
await 0; // force async gap
debugger();
yield 1; // LINE_B.
debugger();
@@ -23,7 +22,6 @@ foobar() async* {
}
helper() async {
await 0; // force async gap
debugger();
print('helper'); // LINE_A.
await for (var i in foobar()) {
@@ -78,13 +76,12 @@ var tests = <IsolateTest>[
expect(asyncStack[2].toString(), contains('helper'));
expect(asyncStack[3].kind, equals(M.FrameKind.asyncSuspensionMarker));
expect(asyncStack[4].toString(), contains('testMain'));
// Line 22.
expect(
await asyncStack[0].location.toUserString(), contains('.dart:$LINE_C'));
// Line 29.
expect(await asyncStack[2].location.toUserString(), contains('.dart:29'));
// Line 35.
expect(await asyncStack[4].location.toUserString(), contains('.dart:35'));
// Line 21.
expect(await asyncStack[0].location.toUserString(), contains('.dart:21'));
// Line 27.
expect(await asyncStack[2].location.toUserString(), contains('.dart:27'));
// Line 30.
expect(await asyncStack[4].location.toUserString(), contains('.dart:33'));
},
];
runtime/observatory/tests/service/pause_on_unhandled_async_exceptions2_test.dart
+1 -2
View File
@@ -10,7 +10,7 @@ import 'package:unittest/unittest.dart';
import 'test_helper.dart';
import 'service_test_common.dart';
const LINE_A = 35;
const LINE_A = 34;
class Foo {}
@@ -20,7 +20,6 @@ doThrow() {
}
asyncThrower() async {
await 0; // force async gap
doThrow();
}
runtime/observatory/tests/service/pause_on_unhandled_async_exceptions_test.dart
+1 -2
View File
@@ -9,7 +9,7 @@ import 'package:unittest/unittest.dart';
import 'test_helper.dart';
import 'service_test_common.dart';
const LINE_A = 35;
const LINE_A = 34;
class Foo {}
@@ -19,7 +19,6 @@ doThrow() {
}
asyncThrower() async {
await 0; // force async gap
doThrow();
}
runtime/tests/vm/dart/causal_stacks/utils.dart
+11 -5
View File
@@ -124,8 +124,11 @@ Future<void> noYields3() async {
Map<int, String> noYieldsMapCausal = {
0: '#0 throwSync ',
1: '#1 noYields3 ',
2: '#2 noYields2 ',
3: '#3 noYields ',
2: '<asynchronous suspension>',
3: '#2 noYields2 ',
4: '<asynchronous suspension>',
5: '#3 noYields ',
4: '<asynchronous suspension>',
// Callers, like doTest and main ..
};
@@ -165,9 +168,11 @@ Map<int, String> mixedYieldsMapCausal = {
0: '#0 throwAsync ',
1: '<asynchronous suspension>',
2: '#1 mixedYields3 ',
3: '#2 mixedYields2 ',
4: '<asynchronous suspension>',
5: '#3 mixedYields ',
3: '<asynchronous suspension>',
4: '#2 mixedYields2 ',
5: '<asynchronous suspension>',
6: '#3 mixedYields ',
7: '<asynchronous suspension>',
// Callers, like doTest and main ..
};
@@ -202,6 +207,7 @@ Map<int, String> syncSuffixMapCausal = {
3: '#2 syncSuffix2 ',
4: '<asynchronous suspension>',
5: '#3 syncSuffix ',
6: '<asynchronous suspension>',
// Callers, like doTest and main ..
};
runtime/tests/vm/dart/sync_async_start_pkg_test_test.dart
-27
View File
@@ -1,27 +0,0 @@
// Copyright (c) 2019, 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.
//
// This test ensures that "pkg:stack_trace" (used by "pkg:test") doesn't break
// when causal async stacks are enabled by dropping frames below a synchronous
// start to an async function.
import "package:test/test.dart";
import "package:stack_trace/src/stack_zone_specification.dart";
import 'dart:async';
void main() {
test("Stacktrace includes sync-starts.", () async {
final st = await firstMethod();
expect("$st", allOf([contains("firstMethod"), contains("secondMethod")]));
});
}
Future<StackTrace> firstMethod() async {
return await secondMethod();
}
Future<StackTrace> secondMethod() async {
return StackTrace.current;
}
runtime/vm/debugger.cc
+17 -15
View File
@@ -2266,9 +2266,8 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() {
return NULL;
}
bool sync_async_end = false;
intptr_t synchronous_stack_trace_length =
StackTraceUtils::CountFrames(thread, 0, async_function, &sync_async_end);
StackTraceUtils::CountFrames(thread, 0, async_function);
// Append the top frames from the synchronous stack trace, up until the active
// asynchronous function. We truncate the remainder of the synchronous
@@ -2305,11 +2304,8 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() {
// Now we append the asynchronous causal stack trace. These are not active
// frames but a historical record of how this asynchronous function was
// activated.
intptr_t frame_skip =
sync_async_end ? StackTrace::kSyncAsyncCroppedFrames : 0;
while (!async_stack_trace.IsNull()) {
for (intptr_t i = frame_skip; i < async_stack_trace.Length(); i++) {
for (intptr_t i = 0; i < async_stack_trace.Length(); i++) {
code_obj = async_stack_trace.CodeAtFrame(i);
if (code_obj.IsNull()) {
break;
@@ -2344,15 +2340,22 @@ DebuggerStackTrace* Debugger::CollectAsyncCausalStackTrace() {
}
}
// Follow the link.
frame_skip = async_stack_trace.skip_sync_start_in_parent_stack()
? StackTrace::kSyncAsyncCroppedFrames
: 0;
async_stack_trace = async_stack_trace.async_link();
}
return stack_trace;
}
#if !defined(DART_PRECOMPILED_RUNTIME)
static bool CheckAndSkipAsync(int skip_sync_async_frames_count,
const String& function_name) {
return (skip_sync_async_frames_count == 2 &&
function_name.Equals(Symbols::_ClosureCall())) ||
(skip_sync_async_frames_count == 1 &&
function_name.Equals(Symbols::_AsyncAwaitCompleterStart()));
}
#endif
DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() {
#if defined(DART_PRECOMPILED_RUNTIME)
// Causal async stacks are not supported in the AOT runtime.
@@ -2404,8 +2407,7 @@ DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() {
if (skip_sync_async_frames_count > 0) {
function_name = function.QualifiedScrubbedName();
if (StackTraceUtils::CheckAndSkipAsync(skip_sync_async_frames_count,
function_name)) {
if (CheckAndSkipAsync(skip_sync_async_frames_count, function_name)) {
skip_sync_async_frames_count--;
} else {
// Unexpected function in synchronous call of async function.
@@ -2455,8 +2457,8 @@ DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() {
if (skip_sync_async_frames_count > 0) {
function_name ^= function.QualifiedScrubbedName();
if (StackTraceUtils::CheckAndSkipAsync(
skip_sync_async_frames_count, function_name)) {
if (CheckAndSkipAsync(skip_sync_async_frames_count,
function_name)) {
skip_sync_async_frames_count--;
} else {
// Unexpected function in sync async call
@@ -2510,8 +2512,8 @@ DebuggerStackTrace* Debugger::CollectAwaiterReturnStackTrace() {
if (skip_sync_async_frames_count > 0) {
function_name ^= function.QualifiedScrubbedName();
if (StackTraceUtils::CheckAndSkipAsync(skip_sync_async_frames_count,
function_name)) {
if (CheckAndSkipAsync(skip_sync_async_frames_count,
function_name)) {
skip_sync_async_frames_count--;
} else {
// Unexpected function in synchronous call of async function.
runtime/vm/object.cc
+2 -21
View File
@@ -21886,14 +21886,6 @@ RawFunction* Closure::GetInstantiatedSignature(Zone* zone) const {
return sig_fun.raw();
}
bool StackTrace::skip_sync_start_in_parent_stack() const {
return raw_ptr()->skip_sync_start_in_parent_stack;
}
void StackTrace::set_skip_sync_start_in_parent_stack(bool value) const {
StoreNonPointer(&raw_ptr()->skip_sync_start_in_parent_stack, value);
}
intptr_t StackTrace::Length() const {
const Array& code_array = Array::Handle(raw_ptr()->code_array_);
return code_array.Length();
@@ -21954,14 +21946,12 @@ RawStackTrace* StackTrace::New(const Array& code_array,
result.set_code_array(code_array);
result.set_pc_offset_array(pc_offset_array);
result.set_expand_inlined(true); // default.
result.set_skip_sync_start_in_parent_stack(false);
return result.raw();
}
RawStackTrace* StackTrace::New(const Array& code_array,
const Array& pc_offset_array,
const StackTrace& async_link,
bool skip_sync_start_in_parent_stack,
Heap::Space space) {
StackTrace& result = StackTrace::Handle();
{
@@ -21974,7 +21964,6 @@ RawStackTrace* StackTrace::New(const Array& code_array,
result.set_code_array(code_array);
result.set_pc_offset_array(pc_offset_array);
result.set_expand_inlined(true); // default.
result.set_skip_sync_start_in_parent_stack(skip_sync_start_in_parent_stack);
return result.raw();
}
@@ -22033,9 +22022,8 @@ const char* StackTrace::ToDartCString(const StackTrace& stack_trace_in) {
// Iterate through the stack frames and create C string description
// for each frame.
intptr_t frame_index = 0;
uint32_t frame_skip = 0;
do {
for (intptr_t i = frame_skip; i < stack_trace.Length(); i++) {
for (intptr_t i = 0; i < stack_trace.Length(); i++) {
code_object = stack_trace.CodeAtFrame(i);
if (code_object.IsNull()) {
// Check for a null function, which indicates a gap in a StackOverflow
@@ -22095,9 +22083,6 @@ const char* StackTrace::ToDartCString(const StackTrace& stack_trace_in) {
}
}
// Follow the link.
frame_skip = stack_trace.skip_sync_start_in_parent_stack()
? StackTrace::kSyncAsyncCroppedFrames
: 0;
stack_trace = stack_trace.async_link();
} while (!stack_trace.IsNull());
@@ -22125,9 +22110,8 @@ const char* StackTrace::ToDwarfCString(const StackTrace& stack_trace_in) {
buffer.Printf("pid: %" Pd ", tid: %" Pd ", name %s\n", OS::ProcessId(),
OSThread::ThreadIdToIntPtr(thread->id()), thread->name());
intptr_t frame_index = 0;
uint32_t frame_skip = 0;
do {
for (intptr_t i = frame_skip; i < stack_trace.Length(); i++) {
for (intptr_t i = 0; i < stack_trace.Length(); i++) {
code = stack_trace.CodeAtFrame(i);
if (code.IsNull()) {
// Check for a null function, which indicates a gap in a StackOverflow
@@ -22169,9 +22153,6 @@ const char* StackTrace::ToDwarfCString(const StackTrace& stack_trace_in) {
}
}
// Follow the link.
frame_skip = stack_trace.skip_sync_start_in_parent_stack()
? StackTrace::kSyncAsyncCroppedFrames
: 0;
stack_trace = stack_trace.async_link();
} while (!stack_trace.IsNull());
runtime/vm/object.h
-18
View File
@@ -9659,23 +9659,6 @@ class StackTrace : public Instance {
RawSmi* PcOffsetAtFrame(intptr_t frame_index) const;
void SetPcOffsetAtFrame(intptr_t frame_index, const Smi& pc_offset) const;
bool skip_sync_start_in_parent_stack() const;
void set_skip_sync_start_in_parent_stack(bool value) const;
// The number of frames that should be cut off the top of an async stack trace
// if it's appended to a synchronous stack trace along a sync-async call.
//
// Without cropping, the border would look like:
//
// <async function>
// ---------------------------
// <asynchronous gap marker>
// <async function>
//
// Since it's not actually an async call, we crop off the last two
// frames when concatenating the sync and async stacktraces.
static constexpr intptr_t kSyncAsyncCroppedFrames = 2;
static intptr_t InstanceSize() {
return RoundedAllocationSize(sizeof(RawStackTrace));
}
@@ -9686,7 +9669,6 @@ class StackTrace : public Instance {
static RawStackTrace* New(const Array& code_array,
const Array& pc_offset_array,
const StackTrace& async_link,
bool skip_sync_start_in_parent_stack,
Heap::Space space = Heap::kNew);
private:
runtime/vm/raw_object.h
-4
View File
@@ -2519,10 +2519,6 @@ class RawStackTrace : public RawInstance {
// False for pre-allocated stack trace (used in OOM and Stack overflow).
bool expand_inlined_;
// Whether the link between the stack and the async-link represents a
// synchronous start to an asynchronous function. In this case, we omit the
// <asynchronous suspension> marker when concatenating the stacks.
bool skip_sync_start_in_parent_stack;
};
// VM type for capturing JS regular expressions.
runtime/vm/stack_trace.cc
+6 -22
View File
@@ -4,15 +4,13 @@
#include "vm/stack_trace.h"
#include "vm/stack_frame.h"
#include "vm/symbols.h"
namespace dart {
// Count the number of frames that are on the stack.
intptr_t StackTraceUtils::CountFrames(Thread* thread,
int skip_frames,
const Function& async_function,
bool* sync_async_end) {
const Function& async_function) {
Zone* zone = thread->zone();
intptr_t frame_count = 0;
StackFrameIterator frames(ValidationPolicy::kDontValidateFrames, thread,
@@ -22,12 +20,8 @@ intptr_t StackTraceUtils::CountFrames(Thread* thread,
Function& function = Function::Handle(zone);
Code& code = Code::Handle(zone);
Bytecode& bytecode = Bytecode::Handle(zone);
String& function_name = String::Handle(zone);
const bool async_function_is_null = async_function.IsNull();
intptr_t sync_async_gap_frames = -1;
ASSERT(async_function_is_null || sync_async_end != NULL);
for (; frame != NULL && sync_async_gap_frames != 0;
frame = frames.NextFrame()) {
for (; frame != NULL; frame = frames.NextFrame()) {
if (!frame->IsDartFrame()) {
continue;
}
@@ -45,24 +39,14 @@ intptr_t StackTraceUtils::CountFrames(Thread* thread,
code = frame->LookupDartCode();
function = code.function();
}
if (sync_async_gap_frames > 0) {
function_name = function.QualifiedScrubbedName();
if (!CheckAndSkipAsync(sync_async_gap_frames, function_name)) {
*sync_async_end = false;
return frame_count;
}
--sync_async_gap_frames;
} else {
frame_count++;
}
frame_count++;
if (!async_function_is_null &&
(async_function.raw() == function.parent_function())) {
sync_async_gap_frames = kSyncAsyncFrameGap;
return frame_count;
}
}
if (!async_function_is_null) {
*sync_async_end = sync_async_gap_frames == 0;
}
// We hit the sentinel.
ASSERT(async_function_is_null);
return frame_count;
}
runtime/vm/stack_trace.h
+2 -26
View File
@@ -8,7 +8,6 @@
#include "vm/allocation.h"
#include "vm/flag_list.h"
#include "vm/object.h"
#include "vm/symbols.h"
namespace dart {
@@ -17,12 +16,10 @@ class StackTraceUtils : public AllStatic {
/// Counts the number of stack frames.
/// Skips over the first |skip_frames|.
/// If |async_function| is not null, stops at the function that has
/// |async_function| as its parent, and records in 'sync_async_end' whether
/// |async_function| was called synchronously.
/// |async_function| as its parent.
static intptr_t CountFrames(Thread* thread,
int skip_frames,
const Function& async_function,
bool* sync_async_end);
const Function& async_function);
/// Collects |count| frames into |code_array| and |pc_offset_array|.
/// Writing begins at |array_offset|.
@@ -45,27 +42,6 @@ class StackTraceUtils : public AllStatic {
StackTrace* async_stack_trace,
Array* async_code_array,
Array* async_pc_offset_array);
// The number of frames involved in a "sync-async" gap: a synchronous initial
// invocation of an asynchronous function. See CheckAndSkipAsync.
static constexpr intptr_t kSyncAsyncFrameGap = 2;
// A synchronous invocation of an async function involves the following
// frames:
// <async function>__<anonymous_closure> (0)
// _Closure.call (1)
// _AsyncAwaitCompleter.start (2)
// <async_function> (3)
//
// For 'skip_sync_async_frames_count' in {1, 2}, checks that 'function_name'
// matches the name of method with that index above.
static bool CheckAndSkipAsync(int skip_sync_async_frames_count,
const String& function_name) {
return (skip_sync_async_frames_count == 2 &&
function_name.Equals(Symbols::_ClosureCall())) ||
(skip_sync_async_frames_count == 1 &&
function_name.Equals(Symbols::_AsyncAwaitCompleterStart()));
}
};
} // namespace dart
tests/language_2/vm/causal_async_exception_stack2_test.dart
+12
View File
@@ -29,8 +29,11 @@ test1() async {
expect(
h.stringContainsInOrder(st.toString(), [
'foo3',
'<asynchronous suspension>',
'foo2',
'<asynchronous suspension>',
'foo',
'<asynchronous suspension>',
'test1',
]),
isTrue);
@@ -44,8 +47,11 @@ test1() async {
expect(
h.stringContainsInOrder(st.toString(), [
'bar3',
'<asynchronous suspension>',
'bar2',
'<asynchronous suspension>',
'bar',
'<asynchronous suspension>',
'test1',
]),
isTrue);
@@ -66,8 +72,11 @@ test2() async {
expect(
h.stringContainsInOrder(st.toString(), [
'foo3',
'<asynchronous suspension>',
'foo2',
'<asynchronous suspension>',
'foo',
'<asynchronous suspension>',
'test2',
]),
isTrue);
@@ -81,8 +90,11 @@ test2() async {
expect(
h.stringContainsInOrder(st.toString(), [
'bar3',
'<asynchronous suspension>',
'bar2',
'<asynchronous suspension>',
'bar',
'<asynchronous suspension>',
'test2',
]),
isTrue);
tests/language_2/vm/causal_async_exception_stack_test.dart
+7 -1
View File
@@ -35,9 +35,11 @@ main() async {
expect(
h.stringContainsInOrder(st.toString(), [
'thrower', '.dart:10', //
'<asynchronous suspension>', //
'generator', '.dart:19', //
'<asynchronous suspension>', //
'foo', '.dart:23', //
'<asynchronous suspension>', //
'main', //
]),
isTrue);
@@ -58,8 +60,11 @@ main() async {
expect(
h.stringContainsInOrder(st.toString(), [
'thrower',
'<asynchronous suspension>',
'main.<anonymous closure>.inner.deep',
'<asynchronous suspension>',
'main.<anonymous closure>.inner',
'<asynchronous suspension>',
'main',
'<asynchronous suspension>',
]),
@@ -73,7 +78,8 @@ main() async {
expect(
h.stringContainsInOrder(st.toString(), [
'thrower', '.dart:10', //
'main.<anonymous closure>', '.dart:71', //
'<asynchronous suspension>', //
'main.<anonymous closure>', '.dart:76', //
]),
isTrue);
}