[vm] Be more lenient when assumptions about _StreamIterator._stateData field is not a Future

Issue flutter/flutter#100441
Issue #48695

TEST=vm/dart{,_2}/flutter_regress_100441_test

Change-Id: Ifdd331dc3b0a2b825a938132808b4021c0af5f71
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/239311
Reviewed-by: Slava Egorov <[email protected]>
Reviewed-by: Alexander Markov <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>

Author: mkustermann

runtime/tests/vm/dart/flutter_regress_100441_test.dart

@@ -0,0 +1,29 @@
+// 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';
+
+StackTrace? stackTrace;
+
+main() async {
+  final values = [];
+  await for (final value in produce()) {
+    values.add(value);
+  }
+  Expect.equals('foo', values.single);
+  Expect.isNotNull(stackTrace!);
+}
+
+Stream<String> produce() async* {
+  await for (String response in produceInner()) {
+    yield response;
+  }
+}
+
+Stream<String> produceInner() async* {
+  yield 'foo';
+  stackTrace = StackTrace.current;
+}

runtime/tests/vm/dart_2/flutter_regress_100441_test.dart

@@ -0,0 +1,31 @@
+// 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';
+
+StackTrace stackTrace;
+
+main() async {
+  final values = [];
+  await for (final value in produce()) {
+    values.add(value);
+  }
+  Expect.equals('foo', values.single);
+  Expect.isNotNull(stackTrace);
+}
+
+Stream<String> produce() async* {
+  await for (String response in produceInner()) {
+    yield response;
+  }
+}
+
+Stream<String> produceInner() async* {
+  yield 'foo';
+  stackTrace = StackTrace.current;
+}

runtime/vm/stack_trace.cc

@@ -83,7 +83,8 @@ CallerClosureFinder::CallerClosureFinder(Zone* zone)
       var_data_field(Field::Handle(zone)),
       state_field(Field::Handle(zone)),
       on_data_field(Field::Handle(zone)),
-      state_data_field(Field::Handle(zone)) {
+      state_data_field(Field::Handle(zone)),
+      has_value_field(Field::Handle(zone)) {
   const auto& async_lib = Library::Handle(zone, Library::AsyncLibrary());
   // Look up classes:
   // - async:
@@ -143,6 +144,9 @@ CallerClosureFinder::CallerClosureFinder(Zone* zone)
   state_data_field =
       stream_iterator_class.LookupFieldAllowPrivate(Symbols::_stateData());
   ASSERT(!state_data_field.IsNull());
+  has_value_field =
+      stream_iterator_class.LookupFieldAllowPrivate(Symbols::_hasValue());
+  ASSERT(!has_value_field.IsNull());
 }
 
 ClosurePtr CallerClosureFinder::GetCallerInFutureImpl(const Object& future) {
@@ -201,9 +205,33 @@ ClosurePtr CallerClosureFinder::FindCallerInAsyncGenClosure(
 
   // If the async* stream is await-for'd:
   if (callback_instance_.GetClassId() == stream_iterator_class.id()) {
-    // _StreamIterator._stateData
-    future_ = Instance::Cast(callback_instance_).GetField(state_data_field);
-    return GetCallerInFutureImpl(future_);
+    // If `_hasValue` is true then the `StreamIterator._stateData` field
+    // contains the iterator's value. In that case we cannot unwind anymore.
+    //
+    // Notice: With correct async* semantics this may never be true: The async*
+    // generator should only be invoked to produce a vaue if there's an
+    // in-progress `await streamIterator.moveNext()` call. Once such call has
+    // finished the async* generator should be paused/yielded until the next
+    // such call - and being paused/yielded means it should not appear in stack
+    // traces.
+    //
+    // See dartbug.com/48695.
+    const auto& stream_iterator = Instance::Cast(callback_instance_);
+    if (stream_iterator.GetField(has_value_field) ==
+        Object::bool_true().ptr()) {
+      return Closure::null();
+    }
+
+    // If we have an await'er for `await streamIterator.moveNext()` we continue
+    // unwinding there.
+    //
+    // Notice: With correct async* semantics this may always contain a Future
+    // See also comment above as well as dartbug.com/48695.
+    future_ = stream_iterator.GetField(state_data_field);
+    if (future_.GetClassId() == future_impl_class.id()) {
+      return GetCallerInFutureImpl(future_);
+    }
+    return Closure::null();
   }
 
   UNREACHABLE();  // If no onData is found we have a bug.

runtime/vm/stack_trace.h

@@ -92,6 +92,7 @@ class CallerClosureFinder {
   Field& state_field;
   Field& on_data_field;
   Field& state_data_field;
+  Field& has_value_field;
 
   DISALLOW_COPY_AND_ASSIGN(CallerClosureFinder);
 };

runtime/vm/symbols.h

@@ -414,6 +414,7 @@ class ObjectPointerVisitor;
   V(_handleMessage, "_handleMessage")                                          \
   V(_handleFinalizerMessage, "_handleFinalizerMessage")                        \
   V(_handleNativeFinalizerMessage, "_handleNativeFinalizerMessage")            \
+  V(_hasValue, "_hasValue")                                                    \
   V(_instanceOf, "_instanceOf")                                                \
   V(_listGetAt, "_listGetAt")                                                  \
   V(_listLength, "_listLength")                                                \

sdk/lib/async/stream_impl.dart

@@ -978,6 +978,7 @@ class _StreamIterator<T> implements StreamIterator<T> {
   /// This will usually cause the [_subscription] to be paused, but as an
   /// optimization, we only pause after the [moveNext] future has been
   /// completed.
+  @pragma("vm:entry-point")
   bool _hasValue = false;
 
   _StreamIterator(final Stream<T> stream)