patterncsharpMinor
MS CA2202: Object can be disposed more then once
Viewed 0 times
oncecanmoreca2202disposedthenobject
Problem
When running Code Analysis on my project I get the following warning:
CA2202 : Microsoft.Usage : Object 'fs' can be disposed more than once in method
I don't see the problem, and the code compiles fine and produces the expected results in all test cases.
If I remove the
object 'fs' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'fs' before all references to it are out of scope.
I am much grateful if you can review my code so that I can learn the proper way of disposing resources.
CA2202 : Microsoft.Usage : Object 'fs' can be disposed more than once in method
I don't see the problem, and the code compiles fine and produces the expected results in all test cases.
If I remove the
fs.Dispose(); statement then Code Analysis instead produces this warning:object 'fs' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'fs' before all references to it are out of scope.
I am much grateful if you can review my code so that I can learn the proper way of disposing resources.
private string ReadFileContent(string filename, Encoding encoding, string category, FileArchive archiveConfig)
{
FileStream fs = null;
StreamReader sr = null;
string fileContent = string.Empty;
_logger.Debug("Reading file content of \"" + category + "\" file \"" + filename + "\"");
try
{
fs = new FileStream(filename, FileMode.Open, FileAccess.ReadWrite, FileShare.None, 64 * 1024, FileOptions.SequentialScan);
fs.Position = 0;
sr = new StreamReader(fs, encoding, true);
fileContent = sr.ReadToEnd();
}
catch (Exception)
{
ArchiveFile(filename, archiveConfig.ErrorFolder, category, _logger);
throw;
}
finally
{
if (sr != null)
sr.Dispose();
if (fs != null)
fs.Dispose();
}
return fileContent;
}Solution
As @amon mentioned, the correct way of referencing
In general you then have the try-catch block around the using statement to catch eventual initialization errors. This means your Code would look somewhat like this:
This also allows you to declare variables only when used.
Finally a small note on
IDisposable Objects in C# is the using-statement.In general you then have the try-catch block around the using statement to catch eventual initialization errors. This means your Code would look somewhat like this:
String fileContent = String.Empty;
_logger.Debug("Debug-String");
try{
using(FileStream fs = new Filestream(filename, FileMode.Open, FileAccess.ReadWrite,
FileShare.None, 64 * 1024, FileOptions.SequentialScan){
fs.Position = 0;
using (StreamReader sr = new Streamreader(fs, encoding, true){
fileContent = sr.ReadToEnd();
}
}
}
catch (Exception){
ArchiveFile(filename, archiveConfig.ErrorFolder, category, _logger);
throw;
}
return fileContent;This also allows you to declare variables only when used.
Finally a small note on
filename... you are using camelCase in all variables but that. Be consistent in your naming schemaCode Snippets
String fileContent = String.Empty;
_logger.Debug("Debug-String");
try{
using(FileStream fs = new Filestream(filename, FileMode.Open, FileAccess.ReadWrite,
FileShare.None, 64 * 1024, FileOptions.SequentialScan){
fs.Position = 0;
using (StreamReader sr = new Streamreader(fs, encoding, true){
fileContent = sr.ReadToEnd();
}
}
}
catch (Exception){
ArchiveFile(filename, archiveConfig.ErrorFolder, category, _logger);
throw;
}
return fileContent;Context
StackExchange Code Review Q#44245, answer score: 4
Revisions (0)
No revisions yet.