Skip to content

Fix nre while cancelling PrepareAsync via cancellation token#4559

Merged
vonzshik merged 5 commits intomainfrom
4209-prepare-async-cancellation-token-nre-fix
Aug 3, 2022
Merged

Fix nre while cancelling PrepareAsync via cancellation token#4559
vonzshik merged 5 commits intomainfrom
4209-prepare-async-cancellation-token-nre-fix

Conversation

@vonzshik
Copy link
Contributor

Fixes #4209

@vonzshik vonzshik marked this pull request as ready for review July 13, 2022 11:27
@vonzshik vonzshik requested a review from roji as a code owner July 13, 2022 11:27
}
catch (OperationCanceledException)
{
// expected exception
Copy link
Member

Choose a reason for hiding this comment

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

Can use Assert.ThrowsAsync

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really. The cancellation can happen before we check the token (in which case, OCE is thrown), or after (no exception).

Copy link
Member

Choose a reason for hiding this comment

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

Right, makes sense

@vonzshik vonzshik enabled auto-merge (squash) August 3, 2022 10:12
@vonzshik vonzshik merged commit 18b7957 into main Aug 3, 2022
@vonzshik vonzshik deleted the 4209-prepare-async-cancellation-token-nre-fix branch August 3, 2022 10:22
vonzshik added a commit that referenced this pull request Aug 3, 2022
@vonzshik
Copy link
Contributor Author

vonzshik commented Aug 3, 2022

Backported to 6.0.6 via 40664dc.

@Bartleby2718
Copy link

@vonzshik Will this be backported to major version 5 as well?

@roji
Copy link
Member

roji commented Feb 25, 2024

@Bartleby2718 that's unlikely, we generally follow the .NET support lifetimes, and like .NET 5.0, it's out of support; this is also far from being a critical bug justifying a backport, it seems.

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.

Cancelling PrepareAsync() closes the connection

3 participants