Home Library Learn Downloads Support Community
Sign in | United States - English | |
2 out of 3 rated this helpful - Rate this topic

CA2202: Do not dispose objects multiple times

Visual Studio 2010

TypeName

DoNotDisposeObjectsMultipleTimes

CheckId

CA2202

Category

Microsoft.Usage

Breaking Change

Non Breaking

A method implementation contains code paths that could cause multiple calls to IDisposable.Dispose or a Dispose equivalent, such as a Close() method on some types, on the same object.

A correctly implemented Dispose method can be called multiple times without throwing an exception. However, this is not guaranteed and to avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.

To fix a violation of this rule, change the implementation so that regardless of the code path, Dispose is called only one time for the object.

Do not suppress a warning from this rule. Even if Dispose for the object is known to be safely callable multiple times, the implementation might change in the future.

Nested using statements (Using in Visual Basic) can cause violations of the CA2202 warning. If the IDisposable resource of the nested inner using statement contains the resource of the outer using statement, the Dispose method of the nested resource releases the contained resource. When this situation occurs, the Dispose method of the outer using statement attempts to dispose its resource for a second time.

In the following example, a Stream object that is created in an outer using statement is released at the end of the inner using statement in the Dispose method of the StreamWriter object that contains the stream object. At the end of the outer using statement, the stream object is released a second time. The second release is a violation of CA2202.

Copy 
using (Stream stream = new FileStream("file.txt", FileMode.OpenOrCreate))
{
    using (StreamWriter writer = new StreamWriter(stream))
    {
        // Use the writer object...
    }
}

To resolve this issue, use a try/finally block instead of the outer using statement. In the finally block, make sure that the stream resource is not null.

Copy 
Stream stream = null;
try
{
    stream = new FileStream("file.txt", FileMode.OpenOrCreate);
    using (StreamWriter writer = new StreamWriter(stream))
    {
        stream = null;
        // Use the writer object...
    }
}
finally
{
    if(stream != null)
        stream.Dispose();
}
Did you find this helpful?
(1500 characters remaining)
Community Content Add
Annotations FAQ
Updated IDispose to include IsDisposed.

The Dispose pattern can be confusing to many novices, and I've seen code where resources are placed in a using statement, but the programmer went ahead and called Dispose (or a similar method -- Close on streams for example) withing the using block. Albeit redundent, this should be perfectly safe code to write, but isn't because using statements cannot check the dispose state of the object.

As someone stated early, the compiler should be able to track resources for us. Currently, it cannot because it will always assume the resources in a using statement will only ever be Disposed by the statement.

One fix would be to add an IsDisposed property on the IDisposable interface. Most implementation of a dispose method rely on some sort of similar bool already. This way, the compiler can check this property at the end of the using statement scoop before it calls Dispose.

History
oddish, but sometimes you need access to Writer source after closing Writer
Some streams need the writer to be closed to get the correct result, something like this:
byte[] result;
using (MemoryStream ms = new MemoryStream())
{
using (BinaryWriter bw = new BinaryWriter(ms, Encoding.UTF8))
{ bw.Write(data);
bw.Flush();
}
result= ms.ToArray();
}

It is a very odd behavior, as data is accessed after an implicit Close (Dispose), but it's the way that work some writers.
History
Fix the source of the problem

Rather than have a zillion people work around this with silly rules why not just fix the compiler? If the rules checker can find it then the compiler can and properly nest the Dispose.

Compilers should manage resources and memory, not humans.

History
CA2202 example fails
The suggested example fails (warning occurs) with this code:

FileStream fs = null;
try
{
fs = new FileStream("somefile.txt", FileMode.OpenOrCreate);
using (BinaryReader br = new BinaryReader(fs))
{
// some logic here.
br.ReadByte();
fs = null;
}
}
finally
{
if (fs != null)
{
fs.Dispose();
fs = null;
}
}

I use VS 2010 and Framework 4.0.
History
RE: RE: Disagreeing with "Disagreeing with code analysis rules"
This is the best place to discuss about this rule because this is where we land when we have this discussion in mind. Please, let us, developers, gather this kind of info and discussions at the closest level of the API documentation rather than 10 links away. Otherwise community content are kinda useless :) $0$0 $0 $0Thanks for having shared your views, fellow developers, about how to consider this rule, this helped to make the right coding decision.$0
History
RE: Disagreeing with "Disagreeing with code analysis rules"

I don't disagree:-) But if you want to get results such as a revision or removal of the rule, posting on the forum or better yet, creating a Connect bug, is the way to go. The code-analysis team is small and over-worked, so a suggestion from the doc writer is not high on their radar. Connect bugs are. I would especially encourage it for this rule.

As the moderator of the code analysis pages, I won't remove these posts, but I would suggest that you write your critique of the rule and then link to a forum question or Connect bug.

Patrick - a doc writer


History
Disagreeing with "Disagreeing with code analysis rules"

As a user, I am glad to have this feedback here. When I saw a violation of this rule, the first thing I did was come to this page to see if anybody else thought the rule was crazy too. While I realize this location is not the ideal for in depth discussions or communicating feedback to the product team, nonetheless I am glad to have some record of this issue here where it is easily visible in the most likely place a user would check to find it.

History
Disagreeing with code analysis rules
The MSDN Library community content is designed primarily to enable MSDN users to provide feedback, extensions and explanations of the documentation. It is not a good way to have issues with the code analysis rules themselves addressed. To discuss the implementation of code analysis rules, post your questions or opinions on the Visual Studio Code Analysis and Code Metrics forum (http://social.msdn.microsoft.com/Forums/en-US/vstscode/). To provide feedback directly to the development team, you can file a bug on the Connect site for Visual Studio 2010 and .NET Framework 4 (https://connect.microsoft.com/VisualStudio).
History
Suggested solution does not dispose
The suggested solution fails to dispose stream (unless an exception occurs).
History
Poor implementation
Fail.

As previously stated, nested using statements should be encouraged rather than a try / catch / finally syntax.


History
Multiple Nested Using Should Be Encouraged
I agree with JoeM27's comment.  Use of multiple nested using statemetn should be encouraged.  It improves readabilities and it works with correct implementations. 
History
CA2202 Fails with mutiple objects
CA2202 fails to properly detect when more than one object of a different type is disposed in the same code path. It thinks the dispose/close method is being executed twice on the same object when in reality the method is being invoked on two different objects. I believe this to be a nusuance check and have removed this check from the project.
History
CA2202 Makes Little Sense

As stated in the "Rule Description" secion of this article, "A correctly implemented Dispose method can be called multiple times without throwing an exception."  This is also mentioned in the article "Implementing Finalize and Dispose to Clean Up Unmanaged Resources" (http://msdn.microsoft.com/en-us/library/b1yfkh5e.aspx).  So an exception will not be thrown if an object is disposed multiple times unless one of these conditions is true:

  1. I have failed to implement a "correctly implemented Dispose method" in my code.  In this case, I should correct the code in that Dispose method. 
  2. Microsoft has failed to implement a "correctly implemented Dispose method" in the CLR.  This would be a horrific mistake on Microsoft's part and I would hope this would never be the case. 
  3. A third party vendor has provided a component that fails to implement a "correctly implemented Dispose method."  In this case, I would think contacting the vendor about the problem and having them fix it would be the correct thing to do. 

In any event, the solution of dropping the outer using statement in favor of the try/finally construct makes the code much less readable.  This is espeically true when there are three or more nested using statements.  It seems ludicrous to write less readable code to account for the case of an incorrectly implemented Dispose method.  Instead, I would think the more readable nested using statements would be preferred, and the try/finally construct can be used for cases where a third party vendor did not implemnt Dispose correctly and will not provide a corrected version.

The bottom line is that this rule is indended to avoid an ObjectDisposedException thrown from a Dispose() method which should never happen.  As such, the existence of CA2202 makes little sense.

History