1

Should I manually dispose System.Net.Mail.MailMessage?

MailMessage implements IDisposable so I assume I should dispose it, but I could not find any examples where someone actually disposes it.

Is doing something like this correct?:

public void Send(MailMessage message)
{
    using(var smtpClient = new SmtpClient(_host))
    {
       smtpClient.Send(message);
       message.Dispose();
    }
}

This seems just little bit wrong, because I'm explicitly using message.Dispose.

FCin
  • 3,804
  • 4
  • 20
  • 49
  • You don't need to manually dispose it, if you can wrap it's usage in a `using` block.. – stuartd Jan 15 '18 at 11:39
  • @stuartd but I'm wrapping `smtpClient` not `message`. Also when I look at debugger I can see that after `using` block, `message` is not disposed (`disposed` field is set to `false`). – FCin Jan 15 '18 at 11:40
  • @FCin ok sorry. I misread _should I manually dispose_ as _should I dispose_.. – stuartd Jan 15 '18 at 11:45
  • 1
    I would recommend that the calling code of `Send(...)` uses the `using(...)`. since the `Send` doesn't own the `message` object. – CodeTherapist Jan 15 '18 at 12:10

1 Answers1

3

Just because others don't do it doesn't make it correct to not dispose it. Since it implements IDisposable it might use unmanaged resources, so dipose it when you are finished with it.

using(var message = new MailMessage(pass arguments ...))
using(var smtpClient = new SmtpClient(_host))
{
   smtpClient.Send(message);
}

The source shows that there are really some managed and unmanaged things to dispose.

This seems just little bit wrong, because I'm explicitly using message.Dispose.

Yes, i agree, calling message.Dispose() seems to be wrong if you don't control the instance(is created and used in code that is not under your control or not even visible). Because then it's possible that you are disposing it when it has to be used afterwards which could be a bug.

Tim Schmelter
  • 450,073
  • 74
  • 686
  • 939
  • I agree, but do you know how I could dispose it when passing already created message to a method instead of creating one inside? – FCin Jan 15 '18 at 11:44
  • @FCin: then the caller should dipose them, best by using the `using`-statement. If you can't modify that code you can't ensure that they are disposed and i would not call `Dispose` here, because those `MailMEssage` instances could still be used there. – Tim Schmelter Jan 15 '18 at 11:45
  • Thank you. I agree, the caller should dispose it. I guess this ensures Single Responsiblity Principle? – FCin Jan 15 '18 at 11:50
  • @FCin: SRP just means that a class should be responsible only for one thing not multiple. If a `MailMessage` would also be responsible for sending mails, then this would be a violation of SRP – Tim Schmelter Jan 15 '18 at 12:20