Skip to content

Recovery loop is not cancelled on connection dispose #1825

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

Open
AndreReise opened this issue Apr 10, 2025 · 1 comment
Open

Recovery loop is not cancelled on connection dispose #1825

AndreReise opened this issue Apr 10, 2025 · 1 comment
Assignees
Labels
Milestone

Comments

@AndreReise
Copy link

AndreReise commented Apr 10, 2025

Describe the bug

If AutorecoveringConnection in recovery state is disposed without calling CancelAsync, the recovery loop never completes because an ObjectDisposedException is thrown, even if RabbitMQ becomes reachable.

Can be reproduced on both the 7.0.0 and 7.1.2 client versions.

Reproduction steps

The following code sample can be used to reproduce the issue.

 ConnectionFactory connectionFactory = new()
 {
     AutomaticRecoveryEnabled = true,
 };

 var connection = await connectionFactory.CreateConnectionAsync();

 var channel = await connection.CreateChannelAsync();

 // Ensure recovery loop is started (breakpoint in RecoverConnectionAsync is hit)
 // Then run "docker restart rabbitmq"
 Console.ReadKey();

 AppDomain.CurrentDomain.FirstChanceException += (_, args) =>
 {
     Console.WriteLine(args.Exception.Message);
 };

 await connection.DisposeAsync();

 await Task.Delay(30_000);
  1. Create and start the RabbitMQ container. Set a breakpoint in AutorecoveringConnection.RecoverConnectionAsync method.
  2. Run docker restart rabbitmq to force a reconnection. This should hit the breakpoint mentioned above.
  3. Press any key in the console to continue execution, which will call DisposeAsync on the connection.
  4. Review the console output.

Sample console output:
Image

Expected behavior

The disconnecting documentation section states:

While disposing channel and connection objects is sufficient, the best practice is that they be explicitly closed first.

I believe the recovery loop should still be properly terminated, even if CancelAsync is not explicitly called.

Additional context

It seems that the fix is to call CancellationTokenSource.Cancel on _recoveryCancellationTokenSource before disposing it, since disposing does not cancel the tokens.

https://github.com/rabbitmq/rabbitmq-dotnet-client/blob/v7.1.2/projects/RabbitMQ.Client/Impl/AutorecoveringConnection.cs#L303

If the maintainers share the vision that the loop should be terminated in any case (and the reported issue is considered a bug), I'd be happy to contribute a fix.

@AndreReise AndreReise added the bug label Apr 10, 2025
@michaelklishin
Copy link
Contributor

Thank you for all these details and even taking a look at what might be the root cause.

@lukebakken lukebakken self-assigned this Apr 12, 2025
@lukebakken lukebakken added this to the 7.1.3 milestone Apr 12, 2025
@lukebakken lukebakken modified the milestones: 7.1.3, 7.2.0 May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants