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

Ensure broker-originated channel closure completes #1752

Merged
merged 1 commit into from
Jan 9, 2025

Conversation

lukebakken
Copy link
Contributor

@lukebakken lukebakken commented Dec 20, 2024

Fixes #1749

  • Ensure Dispose and DisposeAsync are idempotent and thread-safe.
  • Use TaskCompletionSource when HandleChannelCloseAsync runs to allow dispose methods to wait.

@lukebakken lukebakken self-assigned this Dec 20, 2024
@lukebakken lukebakken added this to the 7.1.0 milestone Dec 20, 2024
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1749 branch from 26bc93c to 30a244d Compare December 23, 2024 19:52
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1749 branch 3 times, most recently from b805677 to fd2377b Compare January 7, 2025 00:48
@lukebakken lukebakken marked this pull request as ready for review January 7, 2025 01:43
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1749 branch from fd2377b to c4200ab Compare January 7, 2025 02:31
@lukebakken lukebakken changed the title Ensure DisposeAsync and Dispose are idempotent Ensure broker-originated channel closure completes Jan 7, 2025
@danielmarbach
Copy link
Collaborator

Here are my thoughts #1761. I try to make individual commits so that you can drop them as needed / desired

@danielmarbach danielmarbach mentioned this pull request Jan 7, 2025
11 tasks
Fixes #1749

* Ensure `Dispose` and `DisposeAsync` are idempotent and thread-safe.
* Use TaskCompletionSource when `HandleChannelCloseAsync` runs to allow dispose methods to wait.
* Use `Interlocked` for thread safety.
* I like `_isDisposing` better. So sue me!
* Move the `Interlocked.Exchange` code to a getter, for readability.
* Minor nullable change.
@lukebakken lukebakken force-pushed the rabbitmq-dotnet-client-1749 branch from 27eb038 to 9bbcb5e Compare January 9, 2025 17:43
@lukebakken lukebakken merged commit fdc03ca into main Jan 9, 2025
16 checks passed
@lukebakken lukebakken deleted the rabbitmq-dotnet-client-1749 branch January 9, 2025 20:08
@danielmarbach
Copy link
Collaborator

Do you always remove the co-authors when force pushing? That seems unnecessary to me

@lukebakken
Copy link
Contributor Author

No I don't do anything special when force-pushing. I thought that co-authors were preserved! 😬

@lukebakken
Copy link
Contributor Author

Well crap, I now see that your contribution is lost on that one commit. I'll try to figure out what I may have done differently.

@danielmarbach
Copy link
Collaborator

It's not the end of the world. In my heart I know I was there 😆

@lukebakken
Copy link
Contributor Author

Do you have any idea what I may be doing when I rebase and force-push? This last time, I did the following:

git checkout main
git pull
git checkout rabbitmq-dotnet-client-1749
git rebase -i main
# interactive rebase
git push --force

I squashed all of the commits during the interactive rebase, and I re-worded some of the commit messages. I wonder if it is the re-wording that changes things, or if its the squash? I swear this didn't used to happen 🤔

@danielmarbach
Copy link
Collaborator

Did you preserve the Co-authored-by: stuff when you reworded or did the interactive rebase? If you drop that, the co-author is lost.

@lukebakken
Copy link
Contributor Author

I just read about that. I didn't intentionally remove Co-Authored-By but that's the most likely explanation.

@lukebakken
Copy link
Contributor Author

@danielmarbach - fixed in #1764. I'll hopefully not do that again!

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.

ObjectDisposedException when channel is closed by RabbitMQ due to a channel exception
2 participants