Skip to content

Conversation

@abelbraaksma
Copy link
Member

@abelbraaksma abelbraaksma commented Oct 17, 2022

Fixes: #39

If you create a taskSeq and iterate over it multiple times this should be safe (certainly, there could be a backing of a forward-only stream, but in essence, the wrapper for IAsyncEnumerable provided by TaskSeq<...> in TaskBuilder.fs should on itself be safe to use multiple times).

  • Add bunch of tests ✔️ (done: several minimal repro versions)
  • Investigate ✔️ (done: cause is in GetAsyncEnumerator, the else path)

Not unimportant: the TaskSeq module utility functions that use hand-written enumeration code appear to be sound. The ones that use taskSeq under the hood are not. And when used with an IAsyncEnumerable created with taskSeq, they fail, because of this ☝️.

@abelbraaksma abelbraaksma force-pushed the add-multiple-iteration-tests branch from dbded22 to 1eeaa5a Compare October 17, 2022 01:48
abelbraaksma added a commit that referenced this pull request Oct 17, 2022
…erations comes from TaskSeq<_>.GetAsyncEnumerator
abelbraaksma added a commit that referenced this pull request Oct 17, 2022
…erations comes from TaskSeq<_>.GetAsyncEnumerator
abelbraaksma added a commit that referenced this pull request Oct 17, 2022
…erations comes from TaskSeq<_>.GetAsyncEnumerator
@abelbraaksma abelbraaksma force-pushed the add-multiple-iteration-tests branch from a4cf0ec to 9473460 Compare October 17, 2022 02:08
…is, partially fixed

Issue has to do with the MemberwiseClone() for shadowing the enumerator is not enough to reset the necessary states. Furthermore, after this is sorta fixed, the ValueTask that is used to keep the MoveNext boolean gets accessed twice asynchronously, which is not allowed. It also seems that using 'use' on the taskSeq.GetAsyncEnumerator fails by double disposing. This can probably be considered "by design" but should carefully be considered.
@abelbraaksma
Copy link
Member Author

This PR continues here: #42.

@abelbraaksma abelbraaksma deleted the add-multiple-iteration-tests branch October 29, 2022 00:54
@abelbraaksma abelbraaksma added this to the v0.1.0 milestone Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug fix investigation Requires further or deeper investigation test coverage

Projects

None yet

2 participants