Fix spurious resume when awaiting future in async* code

runtime/lib/core_patch.dart

 // Copyright (c) 2013, 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:math";
 import "dart:typed_data";
 
 // Equivalent of calling FATAL from C++ code.
 _fatal(msg) native "DartCore_fatal";
 
@@ skipping 18 matching lines @@
 
 
 // _AsyncStarStreamController is used by the compiler to implement
 // async* generator functions.
 class _AsyncStarStreamController {
   StreamController controller;
   Function asyncStarBody;
   bool isAdding = false;
   bool onListenReceived = false;
   bool isScheduled = false;
+  bool isSuspendedAtYield = false;
   Completer cancellationCompleter = null;
 
   Stream get stream => controller.stream;
 
   void runBody() {
     isScheduled = false;
+    isSuspendedAtYield = false;

[Lasse Reichstein Nielsen, 2015/04/09 06:35:30]

If I understand this correctly, your possible states are:
- scheduled/running (some behavior depends on whether it's canceled)
- adding
- suspended
- done (reached end of function)
While running, even with async code, the async-star controller isn't invoked.
Only when you reach a yield, yield* or exit of the function will the controller
be used.

At a yield*, the state transitions to "adding". When the addStream completion
Future completes, state transitions to scheduled/running (there is no cancel or
pause check after a yield*, only before).

At a yield:
- if canceled, transition to scheduled (and try to exit).
- if paused, transition to suspended.
- otherwise, transition to scheduled (keep running)

On a resume, if suspended, transition to scheduled.

On a cancel, if suspended, transition to scheduled (and set the "cancelled" flag
so yields start trying to exit).

When exiting function, transition to done.


The boolean flags matches this (I think), but if the code was more explicit
about these states, I think the code would be easier to reason about (at least
for me :).

[hausner, 2015/04/09 17:10:19]

I agree that the code and comments could be clearer about the various states. I
don't have a clear picture of the various events and states that streams,
controllers, listeners etc can be in, so this code is more ad-hoc than I'd like.

     asyncStarBody();
   }
 
   void scheduleGenerator() {
     if (isScheduled || controller.isPaused || isAdding) {
       return;
     }
     isScheduled = true;
     scheduleMicrotask(runBody);
   }
 
   // Adds element to steam, returns true if the caller should terminate
   // execution of the generator.
   //
   // TODO(hausner): Per spec, the generator should be suspended before
   // exiting when the stream is closed. We could add a getter like this:
   // get isCancelled => controller.hasListener;
   // The generator would translate a 'yield e' statement to
   // controller.add(e);
   // suspend;
-  // if (controller.isCanelled) return;
+  // if (controller.isCancelled) return;
   bool add(event) {
     if (!onListenReceived) _fatal("yield before stream is listened to!");
+    if (isSuspendedAtYield) _fatal("unexpected yield");
     // If stream is cancelled, tell caller to exit the async generator.
     if (!controller.hasListener) {
       return true;
     }
     controller.add(event);
     scheduleGenerator();
+    isSuspendedAtYield = true;
     return false;
   }
 
   // Adds the elements of stream into this controller's stream.
   // The generator will be scheduled again when all of the
   // elements of the added stream have been consumed.
   // Returns true if the caller should terminate
   // execution of the generator.
   bool addStream(Stream stream) {
     if (!onListenReceived) _fatal("yield before stream is listened to!");
@@ skipping 40 matching lines @@
                                       onCancel: this.onCancel);
   }
 
   onListen() {
     assert(!onListenReceived);
     onListenReceived = true;
     scheduleGenerator();
   }
 
   onResume() {
-    scheduleGenerator();
+    if (isSuspendedAtYield) {
+      scheduleGenerator();
+    }
   }
 
   onCancel() {
     if (controller.isClosed) {
       return null;
     }
     if (cancellationCompleter == null) {
       cancellationCompleter = new Completer();
       scheduleGenerator();
     }
@@ skipping 48 matching lines @@
       if (isYieldEach) {
         // Spec mandates: it is a dynamic error if the class of [the object
         // returned by yield*] does not implement Iterable.
         yieldEachIterator = (current as Iterable).iterator;
         continue;
       }
       return true;
     }
   }
 }