Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

CA2000 when Returning Disposable Object from Method

I have a factory method that builds objects that implement IDisposable. Ultimately it is the callers that manage the lifetime of the created objects. This design is triggering a bunch of CA2000 errors. Is there something fundamentally incorrect in my design, does it need refactoring, or is it just getting too excited about static code analysis warnings?

The factory method

public static DisposableType BuildTheDisposableType(string param1, int param2)
{
    var theDisposable = new DisposableType();

    // Do some work to setup theDisposable

    return theDisposable
}

A caller

using(var dt = FactoryClass.BuildTheDisposableType("data", 4))
{
   // use dt
}    
like image 298
Brian Triplett Avatar asked Dec 06 '13 20:12

Brian Triplett


3 Answers

You should store it to local variable, and wrap initialization in the try-catch-rethrow block, dispose in case of any exception:

public MyDisposable CreateDisposable()
{
    var myDisposable = new MyDisposable();
    try
    {
        // Additional initialization here which may throw exceptions.
        ThrowException();
    }
    catch
    {
        // If an exception occurred, then this is the last chance to
        // dispose before the object goes out of scope.
        myDisposable.Dispose();
        throw;
    }
    return myDisposable;
}

Try to never leave the disposable object vulnerable to exceptions, when Dispose wouldn't be called

PS: someone previously mentioned to dispose inside the finally - this is obviously wrong - in non-exception path you don't want to call Dispose

like image 91
Mikl X Avatar answered Oct 08 '22 01:10

Mikl X


I would recommend that you suppress the CA2000 warning on each individual factory method, or perhaps on the entire class that contains them (but only if that is the only function of that class).

I further recommend that you include a justification:

[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Reliability",
    "CA2000:Dispose objects before losing scope",
    Justification = "This is a factory method. Caller must dispose")]
like image 32
John Saunders Avatar answered Oct 08 '22 01:10

John Saunders


You're getting the error because the creator of the disposable object isn't managing it. However, there's nothing fundamentally wrong with the design. You are just relying on the consumers to leverage using. Not much different than the current ADO objects for example.

like image 2
Mike Perrenoud Avatar answered Oct 08 '22 01:10

Mike Perrenoud