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

Create cancellation token from timeout #1775

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

lukebakken
Copy link
Contributor

Fixes #1759

When passing a timeout of 0, DisposeAsync would block forever after closing a connection.

This change ensures that the timeout is used in a cancellation token.

@lukebakken lukebakken self-assigned this Jan 24, 2025
@lukebakken lukebakken added this to the 7.1.0 milestone Jan 24, 2025
@lukebakken
Copy link
Contributor Author

@JanEggers - can you test this out?

@lukebakken lukebakken marked this pull request as ready for review January 29, 2025 21:06
@JanEggers
Copy link
Contributor

sure

@JanEggers
Copy link
Contributor

just had a look at your change. is there a reason you put the cancellationtokensource inside the extensions? I think it is better placed inside the method in the connection class itself.

if someone calls the method on the connection and not the extensions there might still be an issue.

@lukebakken
Copy link
Contributor Author

@JanEggers good point, let me look at that.

@JanEggers
Copy link
Contributor

all of my tests are green again with this change, so looks good thx.

Fixes #1759

When passing a timeout of 0, `DisposeAsync` would block forever after closing a connection.

This change ensures that the timeout is used in a cancellation token.
@michaelklishin michaelklishin merged commit 51d1eeb into main Feb 4, 2025
16 checks passed
@michaelklishin michaelklishin deleted the rabbitmq-dotnet-client-1759 branch February 4, 2025 18:18
@lukebakken
Copy link
Contributor Author

Thank you for the reviews @michaelklishin and @danielmarbach!

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.

Disposing Connection after closing it with timeout causes deadlock
4 participants