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

Add TimeOut to Abort call in dispose #1164

Merged
merged 1 commit into from
Mar 5, 2022

Conversation

lukebakken
Copy link
Contributor

@lukebakken lukebakken commented Feb 24, 2022

Supersedes #1075

Fixes #938

cc @CeesKass

Note: this also modifies integration tests to add the test suite name to the connection name. This makes it much easier to know which tests may be having issues by looking at the RabbitMQ logs.

Proposed Changes

While testing my usage of the RabbitMQ.Client library against a blocked RabbitMQ server I encountered an infinitely hanging dispose of my publisher see #938

Types of Changes

What types of changes does your code introduce to this project?
Put an x in the boxes that apply

I'm not sure if this is a breaking change or not, I did not dive deep enough into the Abort procedure to know if anything can break when calling with a timeout

Checklist

Put an x in the boxes that apply. You can also fill these out after creating
the PR. If you're unsure about any of them, don't hesitate to ask on the
mailing list. We're here to help! This is simply a reminder of what we are
going to look for before merging your code.

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

@lukebakken lukebakken added this to the 8.0.0 milestone Feb 24, 2022
@lukebakken lukebakken self-assigned this Feb 24, 2022
@lukebakken lukebakken modified the milestones: 8.0.0, 6.2.5 Feb 24, 2022
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1075-main branch 2 times, most recently from dfcc183 to 5e77460 Compare February 28, 2022 18:15
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1075-main branch from 010a1f2 to ba4be20 Compare March 4, 2022 18:41
@lukebakken lukebakken marked this pull request as ready for review March 4, 2022 18:44
Added TimeOut to Abort call to prevent waiting forever in Dispose

Upped timeout to 15 seconds to prevent false positives

Use constants

Add test name to connection name

Clean up resources correctly in connection recovery tests
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1075-main branch from ba4be20 to 4042bc1 Compare March 4, 2022 18:54
@michaelklishin michaelklishin merged commit 93921e6 into main Mar 5, 2022
@michaelklishin michaelklishin deleted the rabbitmq-dotnet-client-1075-main branch March 5, 2022 13:27
lukebakken added a commit that referenced this pull request Mar 7, 2022
lukebakken added a commit that referenced this pull request Mar 8, 2022
Ports commit 39a9f2b to 6.x branch

See also:

Port more of #1164 to this PR
lukebakken added a commit that referenced this pull request Mar 8, 2022
Ports commit 39a9f2b to 6.x branch

See also:

Port more of #1164 to this PR
@lukebakken lukebakken modified the milestones: 6.2.5, 6.3.0 May 6, 2022
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.

Connection.Abort uses a timeout of infinity by default, potentially blocking Connection.Dispose
3 participants