CODEWITHSUNDEEP
c#.net-4.0
JL.
JL.

Reputation: 81312

C# how to correctly dispose of an SmtpClient?

VS 2010 code analysis reports the following:

Warning 4 CA2000 : Microsoft.Reliability : In method 'Mailer.SendMessage()', object 'client' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'client' before all references to it are out of scope.

My code is :

public void SendMessage()
    {
        SmtpClient client = new SmtpClient();

        client.Send(Message);
        client.Dispose(); 
        DisposeAttachments(); 
    }

How should I correctly dispose of client?

Update: to answer Jons question, here is the dispose attachments functionality:

private void DisposeAttachments()
{
    foreach (Attachment attachment in Message.Attachments)
    {
        attachment.Dispose();
    }
    Message.Attachments.Dispose();
    Message = null; 
}

Last Update full class listing (its short)

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Net.Mail;

public class Mailer
{
    public MailMessage Message
    {
        get;
        set;
    }

    public Mailer(MailMessage message)
    {
        this.Message = message; 
    }

    public void SendMessage()
    {
        using (SmtpClient client = new SmtpClient())
        {
            client.Send(Message);
        }
        DisposeAttachments(); 
    }

    private void DisposeAttachments()
    {
        foreach (Attachment attachment in Message.Attachments)
        {
            attachment.Dispose();
        }
        Message.Attachments.Dispose();
        Message = null; 
    }
}

Upvotes: 37

Views: 38907

Answers (7)

remondo
remondo

Reputation: 388

My original problem is about intermittent sending failures. E.g. First Send() succeeds, 2nd Send() fails, 3rd Send() succeeds. Initially I thought I wasn't disposing properly. So I resorted to using().

Anyways, later I added the UseDefaultCredentials = false, and the Send() finally became stable. I still retained the using() function.

Upvotes: 0

Steven
Steven

Reputation: 172716

The SmtpClient class in .NET 4.0 now implements IDisposable, while the SmtpClient class in .NET 2.0 lacks this interface (as Darin noted). This is a breaking change in the framework and you should take appropriate actions when migrating to .NET 4.0. However, it is possible to mitigate this in your code before migrating to .NET 4.0. Here is an example of such:

var client = new SmtpClient();

// Do not remove this using. In .NET 4.0 SmtpClient implements IDisposable.
using (client as IDisposable)
{
    client.Send(message);
}

This code will compile and run correctly both under .NET 2.0 (+3.0 and 3.5) and under .NET 4.0.

Upvotes: 13

kumar
kumar

Reputation: 1

public void SendMessage()
{
    try
    {
        using (SmtpClient client = new SmtpClient())
        {
            client.Send(Message);
        client.dispose()

        }
    }
    finally
    {
        DisposeAttachments(); 
    }
}

Upvotes: -4

user253984
user253984

Reputation:

I would do something like that:

class Attachments : List<Attachment>, IDisposable
{
  public void Dispose()
  {
    foreach (Attachment a in this)
    {
      a.Dispose();
    }
  }
}

class Mailer : IDisposable
{
  SmtpClient client = new SmtpClient();
  Attachments attachments = new Attachments();

  public SendMessage()
  {
    [... do mail stuff ...]
  }

  public void Dispose()
  {
    this.client.Dispose();
    this.attachments.Dispose();
  }
}


[... somewhere else ...]
using (Mailer mailer = new Mailer())
{
  mailer.SendMail();
}

This would allow reusing the SmtpClient Object if you want to send multiple mails.

Upvotes: 5

Chris S
Chris S

Reputation: 65446

This is the neater solution that will pass the code police test (and dispose will always be called if Send fails):

public void SendMessage()
{
    using (SmtpClient client = new SmtpClient())
    {   
        client.Send(Message);
        DisposeAttachments(); 
    }
}

Upvotes: 3

Jon Skeet
Jon Skeet

Reputation: 1501656

public void SendMessage()
{
    using (SmtpClient client = new SmtpClient())
    {
        client.Send(Message);
    }
    DisposeAttachments(); 
}

That way the client will be disposed even if an exception is thrown during the Send method call. You should very rarely need to call Dispose explicitly - it should almost always be in a using statement.

However, it's not clear how the attachments are involved here. Does your class implement IDisposable itself? If so, that's probably the place to dispose of the attachments which are presumably member variables. If you need to make absolutely sure they get disposed right here, you probably need:

public void SendMessage()
{
    try
    {
        using (SmtpClient client = new SmtpClient())
        {
            client.Send(Message);
        }
    }
    finally
    {
        DisposeAttachments(); 
    }
}

Upvotes: 53

Darin Dimitrov
Darin Dimitrov

Reputation: 1039000

using (SmtpClient client = new SmtpClient())
{
    client.Send(Message);
    DisposeAttachments(); 
}

Interesting - contrary to .NET 3.5, SmtpClient implements IDisposable in .NET 4.0, learning new things every day.

Upvotes: 10

Related Questions