search

rabbitmq / rabbitmq-dotnet-client

RabbitMQ .NET client for .NET Standard 2.0+ and .NET 4.6.2+
https://www.rabbitmq.com/dotnet.html
Other
2.07k stars 576 forks source link

IModel.Dispose() blocks if called from within the ConnectionShutdown event #154

Closed reisenberger closed 8 years ago

reisenberger commented 8 years ago

I am an EasyNetQ user assisting another EasyNetQ user on EasyNetQ/EasyNetQ#522 . The investigation of the issue within EasyNetQ seems clearly to point to the problem that IModel.Dispose() blocks if called from within the ConnectionShutdown event.

We note from 2012 correspondence http://grokbase.com/t/rabbitmq/rabbitmq-discuss/128mwgvyvc/net-rabbitmq-client-issue-deadlock-on-connectionshutdown http://grokbase.com/t/rabbitmq/rabbitmq-discuss/129maqaa6s/imodel-dispose-locks-if-called-during-connectionshutdown-event ... that Emile Joubert suggested:

The channel/IModel will be cleaned up by the library. There is no need to do this from within the ConnectionShutdown event handler, and attempting to do so will lead to the problems you describe.

Does this advice still apply? IE Should EasyNetQ stop calling Dispose() on IModels within the ConnectionShutdown handler?

Or should EasyNetQ follow advice in #49 ? , ie call .Close() on the IModel now, rather than .Dispose() ?

Thanks

michaelklishin commented 8 years ago

@reisenberger please post questions to rabbitmq-users. Yes, IModel.Close is what we expect to be used.

sivesind commented 8 years ago

@michaelklishin: @reisenberger had one more question: Is it theoretically OK to omit .Close() entirely? Emile Joubert advised to do so in 2012, does this still apply?

I've done extensive testing, and calling .Close(), .Dispose() or .SafeDispose() within the ConnectionShutdown event consistently leads to a deadlock and a connection that never recovers. .Close() causes it fastest of all. Omitting .Close() does not lead to broker connection starvation or memory leaks, this suggests Emile Jouberts advise still applies: The Channel/Model/IModel appears to release its own resources. Can you confirm this?

michaelklishin commented 8 years ago

@sivesind ah, this is in the context of a connection shutdown… then yes, you don't want to use Close. Dispose makes sense there although it's basically an equivalent of Abort. Doing nothing should also be fine.

sivesind commented 8 years ago

@michaelklishin: Thanks a lot! This really helped out!

(my practical tests with extensive restarts of RMQ shows that .Dispose() and .SafeDispose() also causes deadlocking in EasyNetQ, but that may well be an EasyNetQ issue)