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

IxAsync.Timeout: propagate timeout cancellation to main source #916

Merged
merged 5 commits into from
Nov 14, 2019

Conversation

akarnokd
Copy link
Collaborator

@akarnokd akarnokd commented Jun 6, 2019

The Timeout operator did not cancel the source if it lost the race against the item timeout. Also the timeout timer was not cancelled if the downstream cancelled the whole sequence.

@bartdesmet
Copy link
Collaborator

LGTM.

The remarks in the code around the case where the timeout delay task wins are along the same lines as I was thinking for Amb in the other PR, namely to abandon the losing task and let its exception go unhandled (thus ending up on global handlers on the task pool).

The following invariant would be great to have:

source.Amb(Throw<T>(new TimeoutException()).Delay(timeout))

behaves identical to

source.Timeout(timeout)

for all aspects of cancellation and error propagation of the loser.

@akarnokd
Copy link
Collaborator Author

akarnokd commented Jun 7, 2019

The following invariant would be great to have:

Okay, but source has to be a single item only for Amb as Timeout plays the race for each source item over and over whereas Amb does it only for the first signal.

Anyway, adding such invariance test requires both PRs to be merged first.

@akarnokd akarnokd merged commit ed9cb74 into dotnet:master Nov 14, 2019
@akarnokd akarnokd deleted the TimeoutCancelLoser branch November 14, 2019 20:22
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.

2 participants