Анализ кода жалуется, что я не удаляю объекты. Что здесь не так?

Рассмотрим этот код

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;
        }

Я просто беру свой тип базы данных и конвертирую в MailMessage. Он отправляется в другую функцию.

Анализ кода говорит мне, что я не утилизирую «msg», что правильно. Но если я сделаю это здесь - я получаю исключение, когда пытаюсь его отправить.

Кроме того, он жалуется на отсутствие утилизации MemoryStream здесь:

msg.Attachments.Add (новое вложение (новый поток памяти (вложение. данные), вложение. имя));

Я понятия не имею, как правильно его утилизировать. Я пробовал разные вещи, но получал исключения при отправке почты с сообщением «Поток закрывается».


c# dispose code-analysis mailmessage
person katit    schedule 19.08.2011    source источник

Ответы (3)


arrow_upward
2
arrow_downward

По сути, вы не должны этого делать — удаление почтового сообщения позже уничтожит каждое вложение, которое уничтожит каждый поток. Кроме того, отказ от удаления MemoryStream, который не используется для удаленного взаимодействия, не принесет никакого вреда.

Я предлагаю вам подавить предупреждение для этого метода.

РЕДАКТИРОВАТЬ: я подозреваю, что вы можете использовать [SuppressMessage] для подавить сообщение.

Обратите внимание, что существует риск того, что какой-то код выкинет код на полпути к методу, поэтому вы никогда не сможете избавиться от сообщения, даже если у вас есть оператор using в вызывающем коде. Если вы действительно обеспокоены, вы можете написать:

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();
        }
    }
}

Хотя лично я бы сказал, что это перебор.

person Jon Skeet    schedule 19.08.2011
comment
@katit: Pass - я не использую анализ кода. Я уверен, что в Интернете есть много инструкций. - person Jon Skeet; 19.08.2011
comment
@katit: отредактировал ответ с предложением по подавлению. - person Jon Skeet; 19.08.2011
comment
Да, спасибо! Это был первый пост в Google о том, как это сделать :) Его легко подавить, щелкнув правой кнопкой мыши это предупреждение в VS и выбрав «Подавить в коде» :) - person katit; 20.08.2011
comment
Это законная уловка инструмента анализа кода, рекомендовать подавление кажется неправильным. - person justin.m.chase; 21.08.2011
comment
@justin.m.chase: По крайней мере, для потока памяти, если его не утилизировать, это не повредит. Какие неуправляемые ресурсы, по вашему мнению, будет выделять MailMessage перед отправкой? Я согласен, что его не следует подавлять вообще, но я думаю, что в данном случае это нормально. - person Jon Skeet; 21.08.2011
comment
@Джон: Хорошо, ха-ха. Наверное, я больше думал об общем случае. Согласно рефлектору, метод Dispose в MailMessage удаляет ряд потоков, включая вложения и тому подобное. Также кажется, что они считаются управляемыми ресурсами, а не неуправляемыми, поэтому вы, вероятно, правы, что в этом случае это не имеет значения. Что делать, если вложения действительно большие? Мой единственный другой аргумент будет заключаться в том, что полезно привыкнуть делать то, что правильно в общем случае. - person justin.m.chase; 21.08.2011
comment
@justin: Если бы они загружались из FileStream, это имело бы смысл ... но с этим было бы сложнее иметь дело. По сути, все, что передает право собственности, так же сложно. - person Jon Skeet; 21.08.2011

arrow_upward
0
arrow_downward

Что касается «не удалять« msg »», единственный способ, который я могу придумать, - это вместо возврата MailMessage передать ссылку на MailMessage. Что-то вроде этого. Не уверен, что это такая хорошая идея.

private void GetMailMessageFromMailItem(ref MailMessage msg, Data.SystemX.MailItem mailItem)
person Jethro    schedule 19.08.2011
comment
Подозреваю, что стрим будет храниться во вложении и только потом читаться. - person Jon Skeet; 19.08.2011
comment
@ Джон Скит, теперь, когда я думаю об этом, поток должен оставаться открытым. - person Jethro; 19.08.2011

arrow_upward
-1
arrow_downward

Создатель одноразового объекта также должен его удалить. Если вы не можете избавиться от сообщения здесь, его следует передать от создателя в другом месте. Анализ кода в этом случае является правильным, и вы можете получить очень неприятные и трудно отлаживаемые утечки, если будете игнорировать эти сообщения.

person justin.m.chase    schedule 19.08.2011
comment
Передача права собственности — очень полезная концепция, с которой, к сожалению, не справляются ни C#, ни Code Analysis. - person Ben Voigt; 20.08.2011
comment
Поэтому его следует избегать. Таким образом, разумно возложить на создателя ответственность за распоряжение своими доступными ресурсами. - person justin.m.chase; 21.08.2011
comment
Создатель не всегда является логическим владельцем. Например, в шаблоне фабрики создатель никогда не является владельцем, и фабрика не может распоряжаться созданным ею ресурсом. Вместо этого должен быть возвращен неиспользуемый ресурс. - person Ben Voigt; 21.08.2011
comment
Если только все одноразовые ресурсы не будут переданы фабрике, а объект, созданный фабрикой, сам по себе не является одноразовым. - person justin.m.chase; 21.08.2011
comment
Если объект, созданный фабрикой, владеет одноразовыми объектами, он должен быть одноразовым, чтобы гарантировать, что они будут удалены в нужное время. - person Ben Voigt; 21.08.2011
comment
Таким образом, он не должен владеть ими, вместо этого они должны быть переданы ему, поскольку его создатель также не может им распоряжаться. Владелец этих одноразовых ресурсов избавится от них в нужное время. Например, передайте поток в фабрику вместо передачи пути. - person justin.m.chase; 21.08.2011
comment
Но часто неестественно, чтобы время жизни каждого объекта ограничивалось временем существования метода, который его создает. У вас, конечно, не может быть этого и одного цикла сообщений. Просто посмотрите на конструктор любого окна WinForms... все подэлементы управления создаются внутри конструктора, но не имеет смысла размещать их внутри конструктора. Компоненты были бы гораздо менее пригодными для повторного использования, если бы их время жизни должно было быть строго вложенным, а не произвольно перекрывающимся. - person Ben Voigt; 21.08.2011