Why do we periodically get an "Asynchronous call already in progress" when calling SmtpClient.Send?

Question

We have some (synchronous) email code that creates a class that creates SmtpClient, and then sends the email. SmtpClient is not reused; however, from time to time we get the following exception:

System.Web.HttpUnhandledException (0x80004005): Exception of type 'System.Web.HttpUnhandledException' was thrown. ---> System.InvalidOperationException: An asynchronous call is already in progress. It must be completed or canceled before you can call this method. at System.Net.Mail.SmtpClient.Send(MailMessage message) at EmailSender.SendMail(MailAddress fromMailAddress, string to, String subject, String body, Boolean highPriority) in ...\EmailSender.cs:line 143 

The code is as follows:

 // ... var emailSender = new EmailSender(); emailSender.SendMail(toEmail, subject, body, true); // emailSender not used past this point // ... public class EmailSender : IEmailSender { private readonly SmtpClient smtp; public EmailSender() { smtp = new SmtpClient(); } public void SendMail(MailAddress fromMailAddress, string to, string subject, string body, bool highPriority) { if (fromMailAddress == null) throw new Exception(); if (to == null) throw new ArgumentException("No valid recipients were supplied.", "to"); // Mail initialization var mailMsg = new MailMessage { From = fromMailAddress, Subject = subject, Body = body, IsBodyHtml = true, Priority = (highPriority) ? MailPriority.High : MailPriority.Normal }; mailMsg.To.Add(to); smtp.Send(mailMsg); } } 

Answer 1

You need to get rid of SmtpClient using Dispose , using or by implementing a one-time template for your EmailSender class (which is more appropriate here because you associate the SmtpClient lifetime with the EmailSender lifetime in the constructor.)

This may solve this exception.

Answer 2

I assume that SmtpClient not intended to send multiple messages at once.

Instead, I would change the class as follows:

 public class EmailSender : IEmailSender { Queue<MailMessage> _messages = new Queue<MailMessage>(); SmtpClient _client = new SmtpClient(); public EmailSender() { } public void SendMail(MailAddress fromMailAddress, string to, string subject, string body, bool highPriority) { if (fromMailAddress == null) throw new ArgumentNullException("fromMailAddress"); if (to == null) throw new ArgumentException("No valid recipients were supplied.", "to"); // Mail initialization var mailMsg = new MailMessage { From = fromMailAddress, Subject = subject, Body = body, IsBodyHtml = true, Priority = (highPriority) ? MailPriority.High : MailPriority.Normal }; mailMsg.To.Add(to); lock (_messages) { _messages.Enqueue(mailMsg); if (_messages.Count == 1) { ThreadPool.QueueUserWorkItem(SendEmailInternal); } } } protected virtual void SendEmailInternal(object state) { while (true) { MailMessage msg; lock (_messages) { if (_messages.Count == 0) return; msg = _messages.Dequeue(); } _client.Send(msg) } } } 

As there is no reason to create a client in the constructor.

I also changed so that the class throws an ArgumentNullException rather than an Exception if fromMailAddress is null. An empty Exception does not say much.

Update

The code now uses a thread pool thread to send (and reusing smtpclient).