2

I have this type of code (pseudo):

IModel channel;
try
{
    channel = connection.CreateModel();
}
catch
{
    channel.dispose();
}

Sometimes, I get time-out exception from the channel.Dispose because of bad connectivity. What will happen if i'll just do channel = null;? Is it so wrong? will it stay alive in the rabbit with no way to reach it?

Mohammed Noureldin
  • 14,913
  • 17
  • 70
  • 99
Yogi_Bear
  • 512
  • 2
  • 10
  • 24

2 Answers2

3

Your description of the problem is correct.

If channel is an unmanaged resource (and most likely it is, because it has a dispose method), then by setting the variable channel to null, you are throwing the reference to it away. So in other words, you are just cutting the only way to communicate with this resource in memory, but anyway it will still reside in memory and take some space (even after forgetting its address in memory by setting the reference to null). This is exactly what is called memory leak.

In case your object is not created frequently and is relatively small, this will not hurt (of course this is a bad design), but as soon as your program grows and creates many memory leaks you will be in a bad situation and you memory will start to run out.

Catching exceptions in Dispose is also not good if you don't really pay attention, because it can lead to leaks also, but with additional difficulty in debugging.

In case you are sure that by retrying to dispose will work somewhen later, then in a catch block in Dispose implementation retry to call Dispose again. Again, you have to be careful here to not end up with a leak with debug difficulties (worse than the original problem).

Actually it is very bad that excpetions are thrown in Dispose, however, sometimes you cannot control 3rd party code and you have to deal with that exception carefully in your Dispose.

Just for your information, yet a better dispose implementation would be using this dispose pattern.

And a final word, which I guess you know. If this resource is unmanaged, then you need to call this dispose not in catch, but finally, without mentioning the possibility to wrap the resource with a using keyword.

Mohammed Noureldin
  • 14,913
  • 17
  • 70
  • 99
0

As a rule of thumb, you should always dispose an object if it implements IDisposable and you are the owner of the object. A detailed explanation why was already given in the other answers.

Normally, you would do this with a using block:

using (var channel = connection.CreateModel())
{
  ...
}

Ideally, Dispose should not throw exceptions. But if it does, you shouldn't just ignore them. Ask yourself: What caused the exception? How can you fix the actual problem?

If it's a particular case that you think you can safely ignore, then make sure to catch only that exception specifically, to make sure you handle any other errors that might occur in the future.

In the case of the channel, it simply calls Abort() in the Dispose to gracefully send a Goodbye, which will fail if the connection is down. So I think in this case, it would be safe to catch and ignore that particular exception.

For a further discussion on what to do with exceptions in Dispose also see this SO post.

marsze
  • 15,079
  • 5
  • 45
  • 61