Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Code analysis complains I'm not disposing objects. What is wrong here?

Consider this code

private MailMessage GetMailMessageFromMailItem(Data.SystemX.MailItem mailItem)
        {

            var msg = new MailMessage();

            foreach (var recipient in mailItem.MailRecipients)
            {
                var recipientX = Membership.GetUser(recipient.UserKey);
                if (recipientX == null)
                {
                    continue;
                }

                msg.To.Add(new MailAddress(recipientX.Email, recipientX.UserName));
            }

            msg.From = new MailAddress(ConfigurationManager.AppSettings["EmailSender"],
                                   ConfigurationManager.AppSettings["EmailSenderName"]);

            msg.Subject = sender.UserName;
            if (!string.IsNullOrEmpty(alias)) msg.Subject += "(" + alias + ")";
            msg.Subject += " " + mailItem.Subject;
            msg.Body = mailItem.Body;
            msg.Body += Environment.NewLine + Environment.NewLine + "To reply via Web click link below:" + Environment.NewLine;
            msg.Body += ConfigurationManager.AppSettings["MailPagePath"] + "?AID=" + ContextManager.AccountId + "&RUN=" + sender.UserName;

            if (mailItem.MailAttachments != null)
            {
                foreach (var attachment in mailItem.MailAttachments)
                {
                    msg.Attachments.Add(new Attachment(new MemoryStream(attachment.Data), attachment.Name));
                }
            }

            return msg;
        }

I'm just taking my database type and converting to MailMessage. It get's sent in another function.

Code analysis tells me I'm not disposing "msg" which is correct. But if I do it here - I get exception when I'm trying to send it.

Also, it complains about not disposing MemoryStream here:

msg.Attachments.Add(new Attachment(new MemoryStream(attachment.Data), attachment.Name));

I have no idea on how to properly dispose it. I tried different things but was getting exceptions when sending mail saying "Stream is closes"

like image 936
katit Avatar asked Nov 13 '22 18:11

katit


1 Answers

Basically you shouldn't - disposing of the mail message later will dispose of each attachment, which will dispose of each stream. Besides, failing to dispose of a MemoryStream which isn't being used in remoting won't do any harm.

I suggest you suppress the warning for this method.

EDIT: I suspect you can use [SuppressMessage] to suppress the message.


Note that there's a risk that some code will throw code half way through the method, so you end up never being able to dispose of the message even if you have a using statement in the calling code. If you're really bothered, you could write:

private MailMessage GetMailMessageFromMailItem(Data.SystemX.MailItem mailItem)
{
    bool success = false;
    var msg = new MailMessage();
    try
    {
        // Code to build up bits of the message
        success = true;
        return msg;
    }
    finally
    {
        if (!success)
        {
            msg.Dispose();
        }
    }
}

Personally I'd say this is overkill though.

like image 186
Jon Skeet Avatar answered Nov 23 '22 23:11

Jon Skeet