Skip to content

Recovery loop is not cancelled on connection dispose #1825

Open
@AndreReise

Description

@AndreReise

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.

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions