Skip to content

Conversation

@quinmars
Copy link
Contributor

@quinmars quinmars commented Jun 5, 2019

As pointed out by @danielcweber the current implementation of ToObservable may violate the contract of IAsyncEnumerator, because it is possible to call DisposeAsync during an on going MoveNextAsync call.

#423 (comment)

Copy link
Contributor Author

@quinmars quinmars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm using now the original logic without catching Rx pipeline exceptions. There are two places, however, where the enumerator could throw without being catched atm.

@danielcweber
Copy link
Collaborator

Review by @bartdesmet still needed? His earlier review has been adressed by @quinmars if I see this correctly.

@quinmars
Copy link
Contributor Author

Somehow, github could not link the commit to @bartholinie. Does any one know how to make that work?

@danielcweber
Copy link
Collaborator

"Jens Chr authored and quinmars committed 12 hours ago". Commit info is preserved, isn't it?

@danielcweber
Copy link
Collaborator

danielcweber commented Nov 20, 2019

The requested change was about the recursion and has been adressed, right? Would merge after checks pass.

@danielcweber danielcweber merged commit 46d757b into dotnet:master Nov 20, 2019
@quinmars
Copy link
Contributor Author

Right the commit info is preserve, but @bartholinie is not listed in the contributors list.

@danielcweber
Copy link
Collaborator

@onovotny @bartdesmet Can we have a Linq.Async / Ix.Async preview that would contain this fix? Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants