Kees
Kees

Reputation: 1418

async/await and the IDisposable interface

I have a class which implements the IDisposable interface to dispose a private variable _MailMessage The same class has a async method that makes use of the private IDisposable variable, namely async public Task<bool> Send My question is: Will the normal IDisposable implementation dispose the private variable after the async method completes? Here is an example of the class I am talking about:

public class Email : IEmail
{
    private readonly IEmailData _EmailData;
    private MailMessage _MailMessage = new MailMessage();

    public Email(IEmailData emailData)
    {
        if (emailData == null)
        {
            throw new ArgumentNullException("emailData");
        }
        if (String.IsNullOrEmpty(emailData.To))
        {
            throw new ArgumentNullException("emailData.To");
        }
        if (String.IsNullOrEmpty(emailData.From))
        {
            throw new ArgumentNullException("emailData.From");
        }
        if (String.IsNullOrEmpty(emailData.FromName))
        {
            throw new ArgumentNullException("emailData.FromName");
        }
        if (String.IsNullOrEmpty(emailData.Subject))
        {
            throw new ArgumentNullException("emailData.Subject");
        }
        if (String.IsNullOrEmpty(emailData.Body))
        {
            throw new ArgumentNullException("emailData.Body");
        }

        _EmailData = emailData;
    }


    async public Task<bool> Send()
    {
        return await Task.Run<bool>(() =>
        {
            using (SmtpClient smtp = new SmtpClient())
            {
                smtp.Send(_MailMessage);
            }
            return true;
        });
    }

    #region "IDisposable implementation"
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }


    ~Email()
    {
        Dispose(false);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            if (_MailMessage != null)
                _MailMessage.Dispose();
        }
    }
    #endregion

}

I have changed the IDisposable implementation according to one of the answer that suggest not to use a destructor:

#region "IDisposable implementation"
public void Dispose()
{
    Dispose(true);
    GC.SuppressFinalize(this);
}

protected virtual void Dispose(bool disposing)
{
    if (disposing)
    {
        if (_MailMessage != null)
            _MailMessage.Dispose();
    }
}
#endregion

Upvotes: 8

Views: 10078

Answers (2)

Hans Passant
Hans Passant

Reputation: 942000

You are doing it pretty fundamentally wrong. A good rule to keep in mind that if you think that you need a destructor then you're wrong 99.9% of the time. A destructor is only required if you have a private variable of an unmanaged type that needs to be released. You don't. The way that you can tell you are doing it wrong is when you find out that you are not actually doing anything at all if the disposing argument is false. Or in other words, the destructor doesn't actually do anything. So it isn't needed. Then you don't need the disposable pattern either.

There's more wrongness, you need to inherit the IDisposable interface to implement your own Dispose() method. You forgot.

Your Dispose() method needs to be called by the client code that creates an instance of your Email class. You cannot call it yourself, you don't know when the client code stops using your Email object. So that's a quick answer to your question, you cannot dispose yourself in the Send() method. There's no guarantee whatsoever that the client code will actually call it. You will have to leave it up to the client code to get it right.

Upvotes: 12

Stephen Cleary
Stephen Cleary

Reputation: 456887

I don't see anywhere you're explicitly disposing _MailMessage other than in Email.Dispose.

async doesn't do anything particularly magical with IDispose; the only thing you have to keep in mind is that async methods may return early.

So if you call it like this:

using (var email = new Email(...))
{
  await email.Send();
}

Then your calling code will (asynchronously) wait for Send to complete before disposing email. But if you call it like this:

Task task;
using (var email = new Email(...))
{
  task = email.Send();
}

Then your calling code will dispose email before Send completes.

Upvotes: 15

Related Questions