Psychomatic Complexity Psychomatic Complexity - 4 years ago 115
C# Question

Exception filter causes CA2000 despite a using statement

The following code is a simplified extract from part of our production code. It calculates the SHA256 hash for a file and returns it as a string, or returns

null
if the file cannot be accessed:

private static string CalculateHash(string fileName)
{
try
{
string result;
using (SHA256CryptoServiceProvider sha256 = new SHA256CryptoServiceProvider())
{
byte[] data = File.ReadAllBytes(fileName);
result = BitConverter.ToString(sha256.ComputeHash(data));
}

Debug.WriteLine("Calculated hash for '" + fileName + "': " + result, 3);
return result;
}
catch (UnauthorizedAccessException ex)
{
Debug.WriteLine("The hash calculation failed: " + ex.Message, 3);
return null;
}
catch (IOException ex)
{
Debug.WriteLine("The hash calculation failed: " + ex.Message, 3);
return null;
}
}


One of our developers recently refactored the code using an exception filter to reduce the duplicate
catch
blocks, so it now looks like this:

private static string CalculateHash(string fileName)
{
try
{
string result;
using (SHA256CryptoServiceProvider sha256 = new SHA256CryptoServiceProvider())
{
byte[] data = File.ReadAllBytes(fileName);
result = BitConverter.ToString(sha256.ComputeHash(data));
}

Debug.WriteLine("Calculated hash for '" + fileName + "': " + result, 3);
return result;
}
catch (Exception ex) when (ex is UnauthorizedAccessException || ex is IOException)
{
Debug.WriteLine("The hash calculation failed: " + ex.Message, 3);
return null;
}
}


However we now get a code analysis warning:

CA2000 - In method 'CalculateHash(string)', call System.IDisposable.Dispose on object 'sha256' before all references to it are out of scope.

As far as I can see, the
SHA256CryptoServiceProvider
is being disposed correctly here, and that will happen whether the exception is caught by the filter or not.

Is this CA2000 a false positive, or has the exception filter created a scenario where the disposal won't happen?

Answer Source

It looks like this is a false-positive and can be suppressed safely.

I have compared the Intermediate Language for both versions of the method. They both show the using statement as a try/finally block which is correctly disposing the object. In fact the IL is identical for both methods, except for the outer catch/exception filter section.

 .try
 {
   IL_0000: newobj       instance void [System.Core]System.Security.Cryptography.SHA256CryptoServiceProvider::.ctor()
   IL_0005: stloc.1   V_1
   .try
   {
     IL_0006: ldarg.0   fileName
     IL_0007: call         unsigned int8[] [mscorlib]System.IO.File::ReadAllBytes(string)
     IL_000c: stloc.2   'buffer [Range(Instruction(IL_000c stloc.2)-Instruction(IL_000e ldloc.2))]'
     IL_000d: ldloc.1   V_1
     IL_000e: ldloc.2   'buffer [Range(Instruction(IL_000c stloc.2)-Instruction(IL_000e ldloc.2))]'
     IL_000f: callvirt     instance unsigned int8[] [mscorlib]System.Security.Cryptography.HashAlgorithm::ComputeHash(unsigned int8[])
     IL_0014: call         string [mscorlib]System.BitConverter::ToString(unsigned int8[])
     IL_0019: stloc.0   'string [Range(Instruction(IL_0019 stloc.0)-Instruction(IL_0026 ldloc.0))]'
     IL_001a: leave.s      IL_0026
   } // end of .try
   finally
   {
     IL_001c: ldloc.1   V_1
     IL_001d: brfalse.s    IL_0025
     IL_001f: ldloc.1   V_1
     IL_0020: callvirt     instance void [mscorlib]System.IDisposable::Dispose()
                        /* ^^ here we can see the Dipose method being called
                         *    in the finally block 
                         */
     IL_0025: endfinally   
   } // end of finally
   IL_0026: ldloc.0   'string [Range(Instruction(IL_0019 stloc.0)-Instruction(IL_0026 ldloc.0))]'
   IL_0027: stloc.3   V_3
   IL_0028: leave.s      IL_0034
 } // end of .try
 // ... catch or exception filter IL code then appears here ...
Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download