A convenient way to analyze code for deleting objects

Question

As part of our Visual Studio 2010 development standards (C # 4.0 primarly), we have included code analysis. As I look through the recently submitted code for a new project, I see a ton

CA2000: Microsoft.Reliability: in the "XYZ" method, the "ABC" object is not located along all the exception paths. Call System.IDisposable. the "ABC" object before all references to it go beyond.

warnings. The problem is that nothing I do removes the warning - and I spent hours cleaning the Internet and trying everything I can.

First, let me be clear that I'm not talking about using a simple block to properly manage a local variable - this is not a problem. In my case, these warnings appear when an object is either returned by a method or assigned to another object inside a method.

Here is sample code that contains four such warnings:

public void MainMethod() { var object1 = CreateFirstObject(); // Warning here var object2 = CreateSecondObject(); // Warning here SomeCollectionProperty.Add(object1); SomeCollectionProperty.Add(object2); } private SomeObject CreateFirstObject() { var theObject = new SomeObject() // Warning here { FirstProperty = "some value", // ... }; return theObject; } private SomeOtherObject CreateSecondObject() { var theObject = new SomeOtherObject() // Warning here { FirstProperty = "a different value", // ... }; return theObject; } 

I commented on the lines in which the warnings occur.

I tried to reorganize both creation methods as described in the MSDN article ( here ), but warnings still appear.

UPDATE I should note that both SomeObject and SomeOtherObject implement IDisposable.

In addition, although object initializers can be a component of the problem, keep in mind that initializers are isolated from two private methods and have nothing to do with warnings in MainMethod.

Can someone show me how to properly implement these methods to resolve CA2000 warnings?

Answer 1

The problem that CA2000 detects in this case is that a one-time instance can be orphaned if an exception occurs before it is passed from the method. For example, the “correct” implementation of CreateFirstObject would look something like this:

 private SomeObject CreateFirstObject() { var theObject = new SomeObject(); try { theObject.FirstProperty = "some value"; } catch { theObject.Dispose(); throw; } return theObject; } 

---

Given what you described regarding your desired behavior for MainMethod, its “correct” implementation might look something like this:

 public void MainMethod() { var object1 = CreateFirstObject(); try { SomeCollectionProperty.Add(object1); var object2 = CreateSecondObject(); try { SomeCollectionProperty.Add(object2); } catch { object2.Dispose(); throw; } } catch { object1.Dispose(); SomeCollectionProperty.Remove(object1); // Not supposed to throw if item does not exist in collection. throw; } } 

Answer 2

One way to get rid of the warning is to suppress it in the code:

 [SuppressMessage( "Microsoft.Reliability", "CA2000:DisposeObjectsBeforeLosingScope", Justification = "Factory method")] 

But this is not a real solution to the problem.

The solution is described here: How to get rid of CA2000 warning when transferring ownership?

The specified link basically states that the object is added to the collection that implements ICollection<T> , but I have not tested it.

Answer 3

What happens if you wrap the returned objects mainly in the use block or implement, finally, destroy the objects?

Do SomeOtherObjects need to implement IDisposable?

Answer 4

It is necessary to implement a template similar to the using block, but disable the deletion of the object in the script where it will be successfully returned. The approach suggested by Nicole Kalinou is reasonable, although I prefer to avoid catching exceptions that will just bubble up. My preferred code expression, given the limitations of C #, would be to use the InitializedSuccessfully flag, and then have a “final” block that would take care of deletion if InitializedSuccessfully was not called.

If the class will contain many IDIsposable objects, and the set of such objects will be fixed after the construction is completed, it may be useful to define the IDisposable manager class, which will contain a list of IDisposable objects. Ask the designers of your class to accept the DisposableManager object as a parameter and put all the objects that it creates into the list it created (it may be useful if your class has an instance method:

  T regDisposable <T> RegDispose (T newThing) where T: IDisposable
 {
   myDisposableManager.Add (newThing);
   return newThing;
 }

To use it, after initializing myDisposableManager, just say something like: var someDisposableField = RegDispose(new someDisposableType()); . Such a scheme will offer two great advantages:

• All the main classes that would have to be executed in the Dispose implementation would be `if (myDisposableManager! = Null) myDisposableManager.Dispose ();` Activating the IDisposable object in the constructor (using RegDispose) would also clear it.
• The code that calls the constructor for the main object may, if the constructor throws an exception, call the Dispose method on the DisposableManager object that it created and passed. This would ensure timely cleaning of the partially created object, which is otherwise impossible.

In vb, it is possible that the constructor of the base class can set the constructor parameter as a field available to field initializers. Thus, one could make good use of the RegDispose pattern in field initializers, as well as in an explicit constructor. In C #, this is not possible. [Threadstatic] fields can be used for this purpose, but some care must be taken to ensure that any such fields that are set are also canceled. The constructor receives a call because of something like a thread threadpool may generate a memory leak. In addition, descending fields cannot be accessed almost as efficiently as regular ones, and I don’t know a single way in C # to avoid re-selecting a stream-static field many times - once for each registered IDisposable object.

Answer 5

The pattern that we use and which clears the warning most of the time,

 DisposableObject retVal; DisposableObject tempVal; try{ tempVal = new DisposableObject(); tempVal.DoStuff(); retVal = tempVal; tempVal = null; } finally{ if (tempVal != null) { tempVal.Dispose();} //could also be writtent tempVal?.Dispose(); } return retVal; 

Unfortunately, there are still cases where the warning does not work, so we delete the warning locally with the justification that the disposable material is covered with a pattern.

This is very briefly mentioned in the Microsoft documentation .