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

Connecting to new app throws stream exception #3302

Closed
kenzieschmoll opened this issue Aug 23, 2021 · 9 comments · Fixed by #3404
Closed

Connecting to new app throws stream exception #3302

kenzieschmoll opened this issue Aug 23, 2021 · 9 comments · Fixed by #3404
Assignees
Milestone

Comments

@kenzieschmoll
Copy link
Member

[ERROR:flutter/lib/ui/ui_dart_state.cc(209)] Unhandled Exception: Bad state: Already cancelled
#0      StreamQueue._checkNotClosed (package:async/src/stream_queue.dart:521:20)
#1      StreamQueue.cancel (package:async/src/stream_queue.dart:390:5)
#2      DdsExtension.onEventWithHistory.<anonymous closure> (package:dds/vm_service_extensions.dart:94:20)
#3      _rootRun (dart:async/zone.dart:1420:47)
#4      _CustomZone.run (dart:async/zone.dart:1328:19)
#5      _FutureListener.handleWhenComplete (dart:async/future_impl.dart:203:18)
#6      Future._propagateToListeners.handleWhenCompleteCallback (dart:async/future_impl.dart:737:39)
#7      Future._propagateToListeners (dart:async/future_impl.dart:793:11)
#8      Future._addListener.<anonymous closure> (dart:async/future_impl.dart:466:9)
#9      _microtaskLoop (dart:async/schedule_microtask.dart:40:21)
#10     _startMicrotaskLoop (dart:async/schedule_microtask.dart:49:5)

CC @bkonyi - is there a certain way we are supposed to be handling / cancelling calls to onEventWithHistory?

@bkonyi
Copy link
Contributor

bkonyi commented Aug 23, 2021

Not that I know of. Is it possible that you're cancelling the returned stream more than once?

@kenzieschmoll
Copy link
Member Author

We listen to the same stream in multiple places in DevTools - does each call to onEventWithHistory return the same stream or a new instance?

@kenzieschmoll
Copy link
Member Author

In DevTools we call .cancel() on the StreamSubscription object we create with stream.listen - we never actually cancel the stream itself.

@kenzieschmoll kenzieschmoll added this to the M33 milestone Aug 31, 2021
@kenzieschmoll
Copy link
Member Author

This is the leading crash right now. 30k+ exceptions in the last 7 days.

@bkonyi
Copy link
Contributor

bkonyi commented Aug 31, 2021

Can you reproduce locally?

@kenzieschmoll
Copy link
Member Author

Yeah. Repro steps are to connect DevTools to a flutter app, then connect to a new app (click device info in bottom right of DevTools -> connect to new app).

@kenzieschmoll
Copy link
Member Author

kenzieschmoll commented Sep 3, 2021

We listen to the stream using our auto_dispose code in DevTools. We then cancel the stream subscription on app disconnect:

. This is the first call to cancel(), making the call to cancel in dds throw the exception: https://github.com/dart-lang/sdk/blob/master/pkg/dds/lib/vm_service_extensions.dart#L94.

Should we not be cancelling our own stream subscription in DevTools for the streams with history? This seems counter-intuitive to how regular streams are handled. Should dds be checking if streamEvents is closed before calling cancel?

@bkonyi
Copy link
Contributor

bkonyi commented Sep 7, 2021

Oh awesome! Thanks for the details. I'll take a closer look today or tomorrow.

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Sep 27, 2021
…same name would throw a StateError

Fixes flutter/devtools#3302

TEST=test/regress_devtools_issue_3302_test.dart

Change-Id: Ica0665c8a48cdb881ecdf7e3985660513c4b98b8
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/214662
Reviewed-by: Kenzie Schmoll <kenzieschmoll@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
@bkonyi
Copy link
Contributor

bkonyi commented Sep 28, 2021

Fixed in package:dds 2.1.3. We just need to bump the version in DevTools now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants