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

multicast: dont cancel upstream sequence when client is cancelled #32

Merged
merged 1 commit into from
Jan 3, 2023

Conversation

twittemb
Copy link
Contributor

@twittemb twittemb commented Jan 3, 2023

Description

This PR makes the multicast operator iterate over the upstream sequence in a dedicated task so that collaborative cancellation does not apply. In a multicast world, the upstream sequence should continue to be iterated by new Tasks when one of its client is cancelled.

Checklist

  • this PR is based on the main branch and is up-to-date, if not please rebase your branch on the top of main
  • the commits inside this PR have explicit commit messages
  • unit tests cover the new feature or the bug fix
  • the CHANGELOG is up-to-date

@twittemb twittemb force-pushed the fix/multicast-upstream-cancellation branch from e192d87 to 74ed024 Compare January 3, 2023 14:04
@twittemb twittemb merged commit 1286bef into main Jan 3, 2023
@Kolos65
Copy link

Kolos65 commented Aug 29, 2023

Hey there @twittemb 🙋‍♂️

So then how do you cancel the upstream sequence or how will it be canceled? This leads to upstream sequences being stuck in the global scoped Task of the multicast sequence.

If you use share() for example to improve performance by not producing the same values of an expensive async sequence, the upstream sequence will never be cancelled properly after all clients are cancelled.

@Kolos65
Copy link

Kolos65 commented Aug 29, 2023

I think upstream sequences should be cancelled somehow if there are no clients and the only reference to them is the capture of the multicast sequence's Task.

lachenmayer added a commit to lachenmayer/AsyncExtensions that referenced this pull request Dec 18, 2023
…lticast-upstream-cancellation"

This reverts commit 1286bef, reversing
changes made to f5187fa.
This pull request was closed.
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 this pull request may close these issues.

2 participants