U4-6504 - SmtpClient Not Properly Disposed, Causing Emails to Fail Periodically

Created by Nicholas Westby 06 Apr 2015, 21:47:19 Updated by Shannon Deminick 25 May 2016, 09:30:51

Relates to: OUR-351

I have some code that is using umbraco.library.SendMail (there appears to be no replacement for this method in the newer namespaces). Periodically, it will fail to send the email and I'll see this error in my logs:

2015-04-06 21:15:01,594 [13] ERROR umbraco.library - [Thread 37] umbraco.library.SendMail: Error sending mail. System.Net.Mail.SmtpException: Service not available, closing transmission channel. The server response was: Timeout waiting for data from client. at System.Net.Mail.MailCommand.CheckResponse(SmtpStatusCode statusCode, String response) at System.Net.Mail.MailCommand.Send(SmtpConnection conn, Byte[] command, MailAddress from, Boolean allowUnicode) at System.Net.Mail.SmtpTransport.SendMail(MailAddress sender, MailAddressCollection recipients, String deliveryNotify, Boolean allowUnicode, SmtpFailedRecipientException& exception) at System.Net.Mail.SmtpClient.Send(MailMessage message) at umbraco.library.SendMail(String FromMail, String ToMail, String Subject, String Body, Boolean IsHtml)

Based on this, the problem is likely that Umbraco's not properly disposing the SmtpClient: http://stackoverflow.com/questions/9279013/system-net-mail-smtpexception-service-not-available-closing-transmission-chann

You can see that here: https://github.com/umbraco/Umbraco-CMS/blob/ded1def8e2e7ea1a4fd0f849cc7a3f1f97cd8242/src/Umbraco.Web/umbraco.presentation/library.cs#L1600

It should be wrapping that in a using statement.

Comments

Nicholas Westby 27 Apr 2016, 02:39:03

Note that the Umbraco codebase actually has two places with improperly disposed SMTP clients:

Since IIS tends to avoid garbage collection for long periods, it is very likely that these will cause issues.


Sebastiaan Janssen 23 May 2016, 06:30:52

For review: https://github.com/umbraco/Umbraco-CMS/commit/43d791f9a5549bf3a8b30800cee515386785d8c0

We should probably centralize this in an EmailService (there's a Security.EmailService currently) but that's a task for the future. Maybe not though, we don't seem to be doing any emailing from the core anyway (apart from translating and sending forgot password mails).


Nicholas Westby 23 May 2016, 15:16:43

@sebastiaan I believe emails are also sent from the core when content events occur. For example, I can subscribe to a content node so that I am emailed whenever a publish occurs.


Shannon Deminick 25 May 2016, 09:30:01

I've confirmed the other place that we send emails is in the NotificationService and the message and SmtpClient are disposed there


Shannon Deminick 25 May 2016, 09:30:47

code looks good, will close


Priority: Major

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.2.4

Due in version: 7.5.0

Sprint: Sprint 16

Story Points:

Cycle: