9

The SmtpClient Class states that instance members are not thread safe. This can be seen if concurrent calls are made to Send or SendAsync. Both methods will throw a InvalidOperationException on the second call if the first has not yet completed.

The method SendMailAsync, introduced in .NET 4.5, does not list InvalidOperationException as a thrown exception. Do the new .NET 4.5 methods implement some sort of queuing? Reflector isn't able to shed any light on the implementation details of this class, so I assume this has been implemented in native methods.

Can multiple threads call the SendMessageAsync method on a shared instance of the SMTP client safely?

brianfeucht
  • 774
  • 8
  • 21
  • 1
    Methods that are not thread-safe are not required to throw an exception if you access them from multiple threads. – svick Apr 08 '13 at 21:00

2 Answers2

13

I'm not sure why using Reflector didn't work for you. If I decompile it, I see the following code:

[HostProtection(SecurityAction.LinkDemand, ExternalThreading=true)]
public Task SendMailAsync(MailMessage message)
{
    TaskCompletionSource<object> tcs = new TaskCompletionSource<object>();
    SendCompletedEventHandler handler = null;
    handler = delegate (object sender, AsyncCompletedEventArgs e) {
        this.HandleCompletion(tcs, e, handler);
    };
    this.SendCompleted += handler;
    try
    {
        this.SendAsync(message, tcs);
    }
    catch
    {
        this.SendCompleted -= handler;
        throw;
    }
    return tcs.Task;
}

As you can see, it's a simple TAP wrapper for SendAsync(). And if SendAsync() throws an exception, SendMailAsync() just rethrows it.

So, the conclusion is that SendMailAsync() is not thread-safe and that its exceptions are underdocumented.

svick
  • 236,525
  • 50
  • 385
  • 514
  • I guess I'm stuck implementing my own queuing mechanism then. Thanks for the response. – brianfeucht Apr 09 '13 at 00:22
  • @brianfeucht Why don't you use multiple instances of `SmtpClient` instead (one for each thread)? – svick Apr 09 '13 at 00:24
  • That is the plan. Create a pool of SmtpClients so that I can control how many connections a machine makes. If a client is available it will be shared. If the pool is completely used the request will be queued for the next available client. – brianfeucht Apr 09 '13 at 15:35
  • The throw statements there will cause the method to fail on the poor caller before he had a chance to call await. Isn't this a bug in MS's code? – Menahem Aug 09 '17 at 16:17
  • @Menahem I wouldn't call it a bug, but it might be better the way you suggest (the caller gets the exception only after `await`ing the `Task`). You might want to consider opening an issue in [the corefx repo](https://github.com/dotnet/corefx). But it might not be changed anyway, because of backwards compatibility. – svick Aug 09 '17 at 19:41
2

As a note (since I don't have enough points to comment), the traditional way to write an asynchronous operation was to use the Asynchronous Programming Model (APM), but today we typically use the Task-based Asynchronous Pattern (TAP) with its async/await keywords. And although it's not unusual to see TAP wrappers around APM methods, it's also possible to see APM wrappers around TAP methods.

Moe Howard
  • 157
  • 9