The Code Analysis-friendly way to dispose of objects

Question

As part of our Visual Studio 2010 (primarly C# 4.0) development standards, we have Code Analysis turned on. As I am reviewing recently submitted code for a new project, I am seeing a ton of

CA2000 : Microsoft.Reliability: In method 'XYZ', object 'ABC' is not disposed along all exception paths. Call System.IDisposable.Dispose on object 'ABC' before all references to it are out of scope.

warnings. The problem is that nothing I do seems to eliminate the warnings - and I've spent hours scouring the web and trying everything I can.

First, let me be clear that I am not talking about putting a simple using block in to properly dispose of a local variable - that's not an issue. In my case, these warnings are appearing when the object is either returned by the method or assigned to another object within the method.

Here is a code sample 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've commented the lines where the warnings occur.

I've tried refactoring both Create methods as described in the MSDN article (here) but the warnings still appear.

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

Also, while object initializers may be a component of the problem, keep in mind that the initializers are isolated to the two private methods and have nothing to do with the warnings in MainMethod.

Can anyone show me how to properly implement these methods to eliminate the CA2000 warnings?

Answer 1

The problem that is being detected by CA2000 in this case is that a disposable instance may be "orphaned" if an exception occurs before it is passed out of the method. For example, a "correct" implementation of CreateFirstObject would look something like the following:

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

    return theObject;
}

---

Given what you have described concerning your desired behaviour 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 code:

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

But this is not a real solution of the problem.

A solution is described here: How to get rid of CA2000 warning when ownership is transferred?

In the mentioned link it is basically stated to add the object to a collection implementing ICollection<T>, but I haven't tested that.