CA2202 Code Anaysis issue with Dispose

Question

I can't get my head around the resolution to these code analysis warnings.

This is the code:

   public static string Crypt(string s_Data, string s_Password, bool b_Encrypt)
    {
        byte[] u8_Salt = new byte[] { ... };

        PasswordDeriveBytes i_Pass = new PasswordDeriveBytes(s_Password, u8_Salt);

        Rijndael i_Alg = Rijndael.Create();
        i_Alg.Key = i_Pass.GetBytes(32);
        i_Alg.IV = i_Pass.GetBytes(16);

        ICryptoTransform i_Trans = (b_Encrypt) ? i_Alg.CreateEncryptor() : i_Alg.CreateDecryptor();

        MemoryStream i_Mem = new MemoryStream();
        CryptoStream i_Crypt = new CryptoStream(i_Mem, i_Trans, CryptoStreamMode.Write);

        byte[] u8_Data;
        if (b_Encrypt)
            u8_Data = Encoding.Unicode.GetBytes(s_Data);
        else
            u8_Data = Convert.FromBase64String(s_Data);

        try
        {
            i_Crypt.Write(u8_Data, 0, u8_Data.Length);
            i_Crypt.Close();
            if (b_Encrypt)
                return Convert.ToBase64String(i_Mem.ToArray());
            else
                return Encoding.Unicode.GetString(i_Mem.ToArray());
        }
        catch
        {
            return null;
        }
        finally
        {
            i_Crypt.Close();
        }
    }

The errors are being raised on the i_Crypt.Close(); call. I can see that I have that call twice in my code.

Errors:

You're disposing it within the body of the `try` as well as in the `finally` - just take out the one in the `try` — stuartd, Nov 21 '16 at 16:27
Just remove i_Crypt.Close() from the try block, as finally is always being executed. — Chris Schmitz, Nov 21 '16 at 16:27
Though disposing an object more than once is a bad code smell that ought to be eliminated, the warning is somewhat misguided. An object which throws ObjectDisposedException on a second dispose has a bug; MSDN documentation says that Dispose must be idempotent. — Eric Lippert, Nov 21 '16 at 17:31

score 3 · Accepted Answer · edited May 23 '17 at 11:59

3

Simply remove the Close(); from your try block.

try
{
    i_Crypt.Write(u8_Data, 0, u8_Data.Length);
    if (b_Encrypt)
        return Convert.ToBase64String(i_Mem.ToArray());
    else
        return Encoding.Unicode.GetString(i_Mem.ToArray());
}
catch
{
    return null;
}
finally
{
    i_Crypt.Close();
}

Even better, if you tried a little bit you could likely use your MemoryStream and the CryptoStream in a using block.

Should I call Close() or Dispose() for stream objects?

Using block vs IDisposabe.Dispose()

edited May 23 '17 at 11:59

Community

1
1

answered Nov 21 '16 at 16:26

Derrick Moeller

4,808
2
22
48

So, you are saying that a `using` block of code will automatically call the `close()` call? So that I am clear. Thanks. – Andrew Truckle Nov 21 '16 at 16:31
2

Actually it is calling the Dispose() method defined by IDisposable, not Close(). I'm unsure if Close() is doing some additional cleanup stuff that Dispose isn't doing, but it might be a difference to use a using block and to call .Close() on the object. – Chris Schmitz Nov 21 '16 at 16:33

CA2202 Code Anaysis issue with Dispose

1 Answers1