Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Async generator stream is not closed if cancelled #25967

Closed
alsemenov opened this issue Mar 10, 2016 · 6 comments
Closed

Async generator stream is not closed if cancelled #25967

alsemenov opened this issue Mar 10, 2016 · 6 comments
Assignees
@lrhn
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async

Comments

@alsemenov
Copy link
Contributor

Dart Programming Language Specification (4th edition draft) version 1.11 chapter 16.14 Function Invocation reads:

If f is marked async* (9), then a fresh instance s implementing the built-in
class Stream is associated with the invocation and immediately returned. When
s is listened to, execution of the body of f will begin. When f terminates:
• If the current return value is defined then, if s has been canceled then its
cancellation future is completed with null (16.2).
• If the current exception x is defined:
– x is added to s.
– If s has been canceled then its cancellation future is completed with
x as an error.
• s is closed.

Please, note, that according to the specification s is closed in all cases.
The description for method Stream.listen() https://api.dartlang.org/1.14.2/dart-async/Stream/listen.html reads:

If this stream closes, the onDone handler is called.

Here is the sample code that demonstrates the problem:

import 'dart:async';

int sum = 0;

Stream generator() async* {
  // do not use yield here, because it may terminate the function
  // just do some actions
  for(int i=0; i<10000; i++){
    sum += i;
  }
}

main() {
  Stream s = generator();
  s.listen(
      (_) {},
      onDone: () {
        print('onDone: sum: $sum (expected: 49995000)');
      }
  ).cancel().then(
      (_) {
        print('cancel future is completed');
  });
}

Output:

cancel future is completed

Expected output:

cancel future is completed
onDone: sum: 49995000 (expected: 49995000)

Dart VM version: 1.14.2 (Tue Feb 09 15:17:24 2016) on "windows_x64"
@alsemenov
Copy link
Contributor Author

The same problem is observed if generator() throws exception instead of normal completion.

@lrhn
Copy link
Member

lrhn commented Mar 10, 2016

When you call cancel on a StreamSubscription, you preclude any further events from being received. That's what cancel means: Stop receiving events. For a single subscription stream, it should also trigger early clean-up because the stream knows that any further events it might generate will be ignored anyway.
The done event is an event, so calling cancel also means that the onDone handler won't be called, since the done event was cancelled.

It might not be clear enough from the StreamSubscription documentation that the calling of onDone is also an event triggered callback, like onData and onError, and that cancel will ensure that no further callbacks are triggered.

@alsemenov
Copy link
Contributor Author

Is there a way to find out the state of the Stream? (i.e. closed or not).
The onDone event was the only one I have found.
The same question is applicable to the Future: is there a way to find out whether it is completed or not?

@lrhn
Copy link
Member

lrhn commented Mar 11, 2016

A Stream doesn't have a state as such (at least not a single-subscription stream).

The StreamSubscription is either active (more events pending), done (done event has been delivered) or cancelled (by user). For a user, the cancelled and done states are indistinguishable - no further events will occur. The code that called cancel will have a received Future that completes eventually, when the stream subscription considers itself properly cancelled.

There is no way to inspect the subscription to see which state it is in, the user is expected to keep track the state of the subscriptions they have control of.

@kevmoo kevmoo added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async labels Mar 13, 2016
@whesse
Copy link
Member

whesse commented Jun 30, 2016

It seems to me that either the spec should be changed or the implementations in the vm and dart2js should be changed. Please label this with vm and dart2js and spec, and explain in more detail what the problem is and what the solutions could be.

@lrhn lrhn self-assigned this Jun 30, 2016
@lrhn
Copy link
Member

lrhn commented Jun 30, 2016

Reading this again - I think everybody is acting correctly, but the documentation should be improved. You only get an onDone call when a stream closes if you haven't already called cancel.

lrhn added a commit that referenced this issue Jul 6, 2016
@lrhn
Update documentation for Stream.listen.
bb96f28 
Addresses issue #25967
BUG= http://dartbug.com/25967
R=floitsch@google.com

Review URL: https://codereview.chromium.org/2119153002.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lrhn lrhn

Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@kevmoo @anders-sandholm @alsemenov @lrhn @whesse