CA2202, как решить этот случай

103

Кто-нибудь может сказать мне, как удалить все предупреждения CA2202 из следующего кода?

public static byte[] Encrypt(string data, byte[] key, byte[] iv)
{
    using(MemoryStream memoryStream = new MemoryStream())
    {
        using (DESCryptoServiceProvider cryptograph = new DESCryptoServiceProvider())
        {
            using (CryptoStream cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write))
            {
                using(StreamWriter streamWriter = new StreamWriter(cryptoStream))
                {
                    streamWriter.Write(data);
                }
            }
        }
        return memoryStream.ToArray();
    }
}

Предупреждение 7 CA2202: Microsoft.Usage: объект «cryptoStream» можно удалить более одного раза в методе «CryptoServices.Encrypt (string, byte [], byte [])». Чтобы избежать генерации исключения System.ObjectDisposedException, не следует вызывать Dispose более одного раза для объекта .: Строки: 34

Предупреждение 8 CA2202: Microsoft.Usage: объект memoryStream может быть удален более одного раза в методе CryptoServices.Encrypt (string, byte [], byte []) ». Чтобы избежать генерации исключения System.ObjectDisposedException, не следует вызывать Dispose более одного раза для объекта .: Строки: 34, 37

Чтобы увидеть эти предупреждения, вам потребуется Visual Studio Code Analysis (это не предупреждения компилятора C #).

Testalino
источник
1
Этот код не генерирует эти предупреждения.
Жюльен Оарау,
1
Я получаю 0 предупреждений об этом (уровень предупреждения 4, VS2010). А для тех, кто ищет проблемы в этой области, просьба также добавить текст предупреждений.
Хенк Холтерман,
29
Предупреждения CAxxxx генерируются Code Analysis и FxCop.
dtb
Это предупреждение не относится к показанному коду - предупреждения могут быть подавлены именно для этого сценария. После того, как вы проверили свой код и согласились с этой оценкой, поместите это над своим методом: « [SuppressMessage("Microsoft.Usage", "CA2202:Do not dispose objects multiple times", Justification="BrainSlugs83 said so.")]» - убедитесь, что у вас есть оператор « using System.Diagnostics.CodeAnalysis;» в вашем блоке использования.
BrainSlugs83
2
Посмотрите: msdn.microsoft.com/en-us/library/…
Адиль Мамедов

Ответы:

-4

Это компилируется без предупреждения:

    public static byte[] Encrypt(string data, byte[] key, byte[] iv)
    {
        MemoryStream memoryStream = null;
        DESCryptoServiceProvider cryptograph = null;
        CryptoStream cryptoStream = null;
        StreamWriter streamWriter = null;
        try
        {
            memoryStream = new MemoryStream();
            cryptograph = new DESCryptoServiceProvider();
            cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write);
            var result = memoryStream;              
            memoryStream = null;
            streamWriter = new StreamWriter(cryptoStream);
            cryptoStream = null;
            streamWriter.Write(data);
            return result.ToArray();
        }
        finally
        {
            if (memoryStream != null)
                memoryStream.Dispose();
            if (cryptograph != null)
                cryptograph.Dispose();
            if (cryptoStream != null)
                cryptoStream.Dispose();
            if (streamWriter != null)
                streamWriter.Dispose();
        }
    }

Изменить в ответ на комментарии: я только что еще раз подтвердил, что этот код не генерирует предупреждение, в то время как исходный. В исходном коде CryptoStream.Dispose()и MemoryStream().Dispose() фактически вызываются дважды (что может быть проблемой, а может и не быть).

Модифицированный код работает следующим образом: ссылки устанавливаются null, как только ответственность за удаление передается другому объекту. Например memoryStream, устанавливается значение nullпосле успешного вызова CryptoStreamконструктора. cryptoStreamустановлен в null, после успешного вызова StreamWriterконструктора. Если не возникает исключения, streamWriterпомещается в finallyблок и, в свою очередь, удаляет CryptoStreamи MemoryStream.

Хенрик
источник
85
-1 Очень плохо создавать уродливый код только под предупреждение, которое нужно подавить .
Жордау,
4
Я согласен с тем, что вы не должны вырезать код для чего-то, что может быть исправлено в какой-то момент в будущем, просто подавите.
peteski
3
Как это решает проблему? CA2202 все еще сообщается, потому что memoryStream все еще может быть удален дважды в блоке finally.
Крис Гесслер,
3
Поскольку CryptoStream вызывает внутри MemoryStream Dispose, его можно вызывать дважды, что и является причиной предупреждения. Я попробовал ваше решение и все еще получаю предупреждение.
Крис Гесслер
2
О боже, вы правы - я не ожидал, что там будет логика очистки, смешанная с вашей ... логикой-логикой ... - это просто странно и загадочно - это определенно умно - но опять же, страшно - пожалуйста, не делайте этого в производственном коде; Для ясности: вы действительно понимаете, что это не проблема, которую можно решить, верно? (Можно избавляться от этих объектов несколько раз.) - Я бы снял голосование против, если бы мог (SO мешает мне, он говорит, что вам нужно отредактировать ответ) - но я бы сделал это только неохотно ... - и серьезно, никогда не делай этого.
BrainSlugs83
142

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

[SuppressMessage("Microsoft.Usage", "CA2202:Do not dispose objects multiple times")]
public static byte[] Encrypt(string data, byte[] key, byte[] iv) {
  using (var memoryStream = new MemoryStream()) {
    using (var cryptograph = new DESCryptoServiceProvider())
    using (var cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write))
    using (var streamWriter = new StreamWriter(cryptoStream)) {
      streamWriter.Write(data);
    }
    return memoryStream.ToArray();
  }
}

ОБНОВЛЕНИЕ: в документации IDisposable.Dispose вы можете прочитать это:

Если метод Dispose объекта вызывается более одного раза, объект должен игнорировать все вызовы после первого. Объект не должен вызывать исключение, если его метод Dispose вызывается несколько раз.

Можно утверждать, что это правило существует для того, чтобы разработчики могли usingразумно использовать это утверждение в каскаде одноразовых предметов, как я показал выше (или, может быть, это просто приятный побочный эффект). Таким же образом, CA2202 бесполезен, и его следует подавлять в рамках проекта. Настоящим виновником может быть неправильная реализация Dispose, и CA1065 должен позаботиться об этом (если это находится в вашей ответственности).

Жордау
источник
15
На мой взгляд, это ошибка в fxcop, это правило просто неверно. Метод dispose никогда не должен генерировать исключение ObjectDisposedException, и если это так, вам следует разобраться с ним в это время, зарегистрировав ошибку автора кода, который реализует этот способ удаления.
justin.m.chase
15
Я согласен с @HansPassant в другом потоке: инструмент выполняет свою работу и предупреждает вас о неожиданных деталях реализации классов. Лично я считаю, что настоящая проблема - это дизайн самих API. То, что вложенные классы по умолчанию предполагают, что можно взять на себя ответственность за другой объект, созданный в другом месте, кажется весьма сомнительным. Я вижу, где это может быть полезно, если полученный объект должен быть возвращен, но выполнение этого предположения по умолчанию кажется нелогичным, а также нарушением обычных шаблонов IDisposable.
BTJ
8
Но msdn не рекомендует подавлять сообщения этого типа. Посмотрите: msdn.microsoft.com/en-us/library/…
Адиль Мамедов
2
Спасибо за ссылку @AdilMammadov, полезная информация, но Microsoft не всегда прав в этих вещах.
Тим Абелл
40

Что ж, это точно, метод Dispose () для этих потоков будет вызываться более одного раза. Класс StreamReader станет «владельцем» cryptoStream, поэтому при удалении streamWriter также будет удален cryptoStream. Точно так же класс CryptoStream берет на себя ответственность за memoryStream.

Это не совсем настоящие ошибки, эти классы .NET устойчивы к множественным вызовам Dispose (). Но если вы хотите избавиться от предупреждения, вам следует отказаться от оператора using для этих объектов. И немного огорчите себя, рассуждая о том, что произойдет, если код выдаст исключение. Или отключите предупреждение с помощью атрибута. Или просто проигнорируйте предупреждение, потому что это глупо.

Ганс Пассан
источник
10
Необходимость иметь специальные знания о внутреннем поведении классов (например, одноразовое приобретение владения другим) - это слишком много, чтобы спрашивать, хотите ли вы разработать многоразовый API. Так что я считаю, что usingзаявления следует оставить. Эти предупреждения действительно глупы.
Жордау,
4
@ Jordão - разве не для этого нужен инструмент? Чтобы предупредить вас о внутреннем поведении, о котором вы могли не знать?
Ханс Пассан
8
Я согласен. Но я все равно не стал бы отказываться от usingзаявлений. Просто кажется неправильным полагаться на другой объект, чтобы избавиться от объекта, который я создал. Для этого кода, это нормально, но вы много реализаций Streamи TextWriterтам (не только на BCL). Код для их использования должен быть согласованным.
Жордау,
3
Да, согласен с Жордау. Если вы действительно хотите, чтобы программист знал о внутреннем поведении api, выскажитесь, назвав свою функцию DoSomethingAndDisposeStream (Stream stream, OtherData data).
ZZZ
5
@HansPassant Можете ли вы указать, где документировано, что XmlDocument.Save()метод будет вызывать Disposeуказанный параметр? я не вижу этого ни в документации Save(XmlWriter)(где я столкнулся с ошибкой FxCop), ни в самом Save()методе, ни в самой документации XmlDocument.
Ian Boyd
9

Когда StreamWriter удаляется , он автоматически удаляет упакованный Stream (здесь: CryptoStream ). CryptoStream также автоматически удаляет упакованный Stream (здесь: MemoryStream ).

Таким образом, ваш MemoryStream удаляется как CryptoStream, так и оператором using . И ваш CryptoStream расположен в StreamWriter и внешней помощи заявления.


После некоторых экспериментов кажется, что полностью избавиться от предупреждений невозможно. Теоретически MemoryStream необходимо удалить, но тогда теоретически вы больше не сможете получить доступ к его методу ToArray. Практически, MemoryStream не нужно удалять, поэтому я бы выбрал это решение и подавил предупреждение CA2000.

var memoryStream = new MemoryStream();

using (var cryptograph = new DESCryptoServiceProvider())
using (var writer = new StreamWriter(new CryptoStream(memoryStream, ...)))
{
    writer.Write(data);
}

return memoryStream.ToArray();
dtb
источник
"тогда теоретически вы больше не можете получить доступ к его методу ToArray" Итак, переместите returnстроку перед закрывающей скобкой usingобласти.
Бен Фойгт,
9

Я бы сделал это с помощью #pragma warning disable.

Рекомендации .NET Framework рекомендуют реализовать IDisposable.Dispose таким образом, чтобы его можно было вызывать несколько раз. Из описания MSDN для IDisposable.Dispose :

Объект не должен вызывать исключение, если его метод Dispose вызывается несколько раз.

Поэтому предупреждение кажется почти бессмысленным:

Чтобы избежать генерации исключения System.ObjectDisposedException, не следует вызывать Dispose более одного раза для объекта.

Думаю, можно было бы возразить, что предупреждение может быть полезно, если вы используете плохо реализованный объект IDisposable, который не соответствует стандартным рекомендациям по реализации. Но при использовании классов из .NET Framework, как это делаете вы, я бы сказал, что безопасно подавить предупреждение с помощью #pragma. И ИМХО, это предпочтительнее, чем проходить через обручи, как это предлагается в документации MSDN для этого предупреждения .

Джо
источник
4
CA2202 - это предупреждение анализа кода, а не предупреждение компилятора. #pragma warning disableможет использоваться только для подавления предупреждений компилятора. Чтобы подавить предупреждение анализа кода, вам необходимо использовать атрибут.
Martin Liversage
2

Я столкнулся с аналогичными проблемами в своем коде.

Похоже, что все CA2202 запускается, потому что MemoryStreamможет быть удалено, если в конструкторе возникает исключение (CA2000).

Это можно решить так:

 1 public static byte[] Encrypt(string data, byte[] key, byte[] iv)
 2 {
 3    MemoryStream memoryStream = GetMemoryStream();
 4    using (DESCryptoServiceProvider cryptograph = new DESCryptoServiceProvider())
 5    {
 6        CryptoStream cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write);
 7        using (StreamWriter streamWriter = new StreamWriter(cryptoStream))
 8        {
 9            streamWriter.Write(data);
10            return memoryStream.ToArray();
11        }
12    }
13 }
14
15 /// <summary>
16 /// Gets the memory stream.
17 /// </summary>
18 /// <returns>A new memory stream</returns>
19 private static MemoryStream GetMemoryStream()
20 {
21     MemoryStream stream;
22     MemoryStream tempStream = null;
23     try
24     {
25         tempStream = new MemoryStream();
26
27         stream = tempStream;
28         tempStream = null;
29     }
30     finally
31     {
32         if (tempStream != null)
33             tempStream.Dispose();
34     }
35     return stream;
36 }

Обратите внимание, что мы должны вернуть memoryStreamвнутри последнего usingоператора (строка 10), потому что он cryptoStreamудаляется в строке 11 (потому что он используется в streamWriter usingоператоре), что приводит memoryStreamк тому, что также удаляется в строке 11 (потому что memoryStreamиспользуется для создания cryptoStream).

По крайней мере, этот код работал у меня.

РЕДАКТИРОВАТЬ:

Как бы забавно это ни звучало, я обнаружил, что если вы замените GetMemoryStreamметод следующим кодом,

/// <summary>
/// Gets a memory stream.
/// </summary>
/// <returns>A new memory stream</returns>
private static MemoryStream GetMemoryStream()
{
    return new MemoryStream();
}

вы получите тот же результат.

Джими
источник
1

Криптопоток основан на потоке памяти.

Похоже, что происходит то, что, когда крипопоток удаляется (в конце использования), поток памяти также удаляется, затем поток памяти удаляется снова.

Шираз Бхайджи
источник
1

Я хотел решить эту проблему правильно - без подавления предупреждений и правильной утилизации всех одноразовых предметов.

Я извлек 2 из 3 потоков как поля и поместил их в Dispose()метод своего класса. Да, реализация IDisposableинтерфейса не обязательно может быть тем, что вы ищете, но решение выглядит довольно чистым по сравнению с dispose()вызовами из всех случайных мест в коде.

public class SomeEncryption : IDisposable
    {
        private MemoryStream memoryStream;

        private CryptoStream cryptoStream;

        public static byte[] Encrypt(string data, byte[] key, byte[] iv)
        {
             // Do something
             this.memoryStream = new MemoryStream();
             this.cryptoStream = new CryptoStream(this.memoryStream, encryptor, CryptoStreamMode.Write);
             using (var streamWriter = new StreamWriter(this.cryptoStream))
             {
                 streamWriter.Write(plaintext);
             }
            return memoryStream.ToArray();
        }

       public void Dispose()
        { 
             this.Dispose(true);
             GC.SuppressFinalize(this);
        }

       protected virtual void Dispose(bool disposing)
        {
            if (disposing)
            {
                if (this.memoryStream != null)
                {
                    this.memoryStream.Dispose();
                }

                if (this.cryptoStream != null)
                {
                    this.cryptoStream.Dispose();
                }
            }
        }
   }
Дивяншм
источник
0

Не по теме, но я бы посоветовал вам использовать другую технику форматирования для группировки using:

using (var memoryStream = new MemoryStream())
{
    using (var cryptograph = new DESCryptoServiceProvider())
    using (var encryptor = cryptograph.CreateEncryptor(key, iv))
    using (var cryptoStream = new CryptoStream(memoryStream, encryptor, CryptoStreamMode.Write))
    using (var streamWriter = new StreamWriter(cryptoStream))
    {
        streamWriter.Write(data);
    }

    return memoryStream.ToArray();
}

Я также рекомендую использовать varздесь s, чтобы избежать повторения действительно длинных имен классов.

PS Спасибо @ShellShock за указание, что я не могу сначала опустить фигурные скобки, usingкак это было бы memoryStreamв returnзаявлении за пределами области действия.

Дан Абрамов
источник
5
Не будет ли memoryStream.ToArray () вне области видимости?
Polyfun,
Это абсолютно эквивалентно исходному фрагменту кода. Я просто опустил фигурные скобки, так же как вы можете сделать это с помощью ifs (хотя я бы не советовал эту технику ни для чего, кроме usings).
Дэн Абрамов
2
В исходном коде memoryStream.ToArray () находился внутри области действия первого использования; у вас это выходит за рамки.
Polyfun
Большое вам спасибо, я только что понял, что вы имели в виду returnзаявление. Это точно. Я отредактировал ответ, чтобы отразить это.
Дэн Абрамов
Я лично считаю, что usingотсутствие скобок делает код более хрупким (подумайте о годах различий и слияний). joelonsoftware.com/2005/05/11/making-wrong-code-look-wrong & imperialviolet.org/2014/02/22/applebug.html
Тим Абелл
0

Избегайте любого использования и используйте вложенные вызовы Dispose!

    public static byte[] Encrypt(string data, byte[] key, byte[] iv)
    {
        MemoryStream memoryStream = null;
        DESCryptoServiceProvider cryptograph = null;
        CryptoStream cryptoStream = null;
        StreamWriter streamWriter = null;

        try
        {
            memoryStream = new MemoryStream();
            cryptograph = new DESCryptoServiceProvider();
            cryptoStream = new CryptoStream(memoryStream, cryptograph.CreateEncryptor(key, iv), CryptoStreamMode.Write);
            streamWriter = new StreamWriter(cryptoStream);

            streamWriter.Write(data);
            return memoryStream.ToArray();
        }
        finally 
        {
            if(streamWriter != null)
                streamWriter.Dispose();
            else if(cryptoStream != null)
                cryptoStream.Dispose();
            else if(memoryStream != null)
                memoryStream.Dispose();

            if (cryptograph != null)
                cryptograph.Dispose();
        }
    }
Гарри Зальцман
источник
1
Пожалуйста, объясните, почему вам следует избегать usingв этом случае.
StuperUser
1
Вы можете оставить оператор using посередине, но вам нужно разрешить другие. Чтобы получить логически связное и во всех направлениях обновляемое решение, я решил удалить все использования!
Гарри Зальцман
-1

Я использовал такой код, который принимает byte [] и возвращает byte [] без использования потоков.

public static byte[] Encrypt(byte[] data, byte[] key, byte[] iv)
{
  DES des = new DES();
  des.BlockSize = 128;
  des.Mode = CipherMode.CBC;
  des.Padding = PaddingMode.Zeros;
  des.IV = IV
  des.Key = key
  ICryptoTransform encryptor = des.CreateEncryptor();

  //and finaly operations on bytes[] insted of streams
  return encryptor.TransformFinalBlock(plaintextarray,0,plaintextarray.Length);
}

Таким образом, все, что вам нужно сделать, это преобразовать строку в byte [] с использованием кодировок.

Лука Ране
источник
-1

Я просто хотел развернуть код, чтобы мы могли видеть несколько вызовов Disposeобъектов. Реальность такова , что вы являетесь вызовом Disposeна объектах дважды:

memoryStream = new MemoryStream()
cryptograph = new DESCryptoServiceProvider()
cryptoStream = new CryptoStream()
streamWriter = new StreamWriter()

memoryStream.Dispose(); //implicitly owned by cryptoStream
cryptoStream.Dispose(); //implicitly owned by streamWriter
streamWriter.Dispose(); //through a using

cryptoStream.Dispose(); //INVALID: second dispose through using
cryptograph.Dispose(); //through a using
memorySTream.Dipose(); //INVALID: second dispose through a using

return memoryStream.ToArray(); //INVALID: accessing disposed memoryStream

Хотя большинство классов .NET (мы надеемся) устойчивы к ошибкам множественных вызовов .Dispose, не все классы так же защищают от неправильного использования программистами.

FX Cop знает об этом и предупреждает вас.

У вас есть несколько вариантов;

  • вызывать только Disposeодин раз для любого объекта; не использоватьusing
  • продолжайте дважды вызывать dispose и надеяться, что код не выйдет из строя
  • подавить предупреждение
Ян Бойд
источник