Calling Dispose on AutorecoveringModel that has already shutdown throws NullRefException

[stebet]

Just hit this issue during testing in one our systems, where we try to restart a consumer if it has shutdown. For good measure, we decided to call .Dispose on the Model in the consumer, which threw an exception in the AutorecoveringModel.Abort() method.

"Object reference not set to an instance of an object." at RabbitMQ.Client.Impl.AutorecoveringModel.Abort()
   at RabbitMQ.Client.Impl.AutorecoveringModel.System.IDisposable.Dispose()
   at CCP.Protobuf.RabbitMQ.EveRabbitMQProtoSubscriber.StartEventSubscriber()
   at CCP.Protobuf.RabbitMQ.EveRabbitMQProtoSubscriber.OnEventConsumerShutdown(Object sender, ShutdownEventArgs event)

Calling .Dispose should never throw an exception, as it is often called in finally blocks. See here: https://docs.microsoft.com/en-us/visualstudio/code-quality/ca1065?view=vs-2019

A System.IDisposable.Dispose method should not throw an exception. Dispose is often called as part of the cleanup logic in a finally clause. Therefore, explicitly throwing an exception from Dispose forces the user to add exception handling inside the finally clause.

The Dispose(false) code path should never throw exceptions, because Dispose is almost always called from a finalizer.

[lukebakken]

@stebet I'm pretty sure the changes in #832 fix this issue as AutoRecoveringModel now keeps track of its state via the _disposed field. Calling Dispose multiple times won't trigger this error.

I'll close this for now. If you can still reproduce using your own build or an RC of 6.1.0, let me know.

[dfiialko]

I still have the same issue even though I've updated RabbitMQ to 6.2.1

[egil]

Still an issue in 6.8.1 as far as I can tell:

  Message: 
System.NullReferenceException : Object reference not set to an instance of an object.

  Stack Trace: 
AutorecoveringModel.Abort()
AutorecoveringModel.Dispose(Boolean disposing)
IDisposable.Dispose()
...

[lukebakken]

@egil 8.0.1 is not a valid version for this library, could you please double-check that?

[egil]

@egil 8.0.1 is not a valid version for this library, could you please double-check that?

Sorry, mistook the aspire package for this one. Fixed in my original comment.

[lukebakken]

@stebet @egil

I have added a test to demonstrate that this issue no longer exists in main, which will become version 7.0.0 this month.

#1590

[lukebakken]

I've re-opened this issue, but fixes for the 6.x version of this library will probably be limited to only very serious bugs, unless a community user wishes to submit a PR.

[egil]

The workaround (try/catch) works well enough until v7 is out.