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?
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 ...