IDisposable created within a method and returned

idisposable c# best practices
idisposable pattern
c# using dispose
when to use idisposable
return idisposable c#
how to implement idisposable c#
when dispose method is called in c#
how to dispose class object in c#

I happy coded quite a project that works fine and do not manifest any oddities at runtime. So I've decided to run static code analysis tool (I'm using Visual Studio 2010). It came out that rule CA2000 is being violated, message as follows:

Warning - CA2000 : Microsoft.Reliability : In method 'Bar.getDefaultFoo()', call System.IDisposable.Dispose on object 'new Foo()' before all references to it are out of scope.

The code refered goes like this:

private static IFoo getDefaultFoo()
{
    return (Baz.canIDoIt()) ? new Foo() : null;
}

I thought myself: maybe conditional expression spoils the logic (mine or validator's). Changed to this:

private static IFoo getDefaultFoo()
{
    IFoo ret = null;
    if (Baz.canIDoIt())
    {
        retFoo = new Foo();
    }
    return ret;
}

Same thing happened again, but now the object was referred to as retFoo. I've googled, I've msdn'ed, I've stackoverflow'ed. Found this article. There are no operations I need done after creating the object. I only need to return the reference to it. However, I have tried to apply the pattern suggested in OpenPort2 example. Now code looks like this:

private static IFoo getDefaultFoo()
{
    Foo tempFoo = null;
    Foo retFoo = null;
    try
    {
        if (Baz.canIDoIt())
        {
            tempFoo = new Foo();
        }
        retFoo= tempFoo;
        tempFoo = null;
    }
    finally
    {
        if (tempFoo != null)
        {
            tempFoo.Dispose();
        }
    }
    return retFoo;
}

Same message again, but tempFoo variable is rule violator this time. So basically, code went twisted, longer, little irrational, unnecesarrily complex and does the very same, but slower.

I've also found this question, where same rule seems to attack a valid code in similar manner. And there questioner is adviced to ignore the warning. I've also read this thread and a mass of similar questions.

Is there anything I missed? Is the rule bugged / irrelevant? What should I do? Ignore? Handle in some magickal way? Maybe apply some design pattern?

Edit:

After Nicole's request, I'm submiting the whole related code in form I also tried of using.

public class DisposableFooTest
{
    public interface IFoo
    {
        void bar();
    }

    public class Foo : IFoo, IDisposable
    {
        public void bar()
        {
            Console.Out.WriteLine("Foo baring now");
        }

        public void Dispose()
        {
            // actual Dispose implementation is irrelevant, or maybe it is?
            // anyway I followed microsoft dispose pattern
            // with Dispose(bool disposing)
        }
    }

    public static class Baz
    {
        private static bool toggle = false;
        public static bool canIDoIt()
        {
            toggle ^= true;
            return toggle;
        }
    }

    private static IFoo getDefaultFoo()
    {
        IFoo result = null;
        try
        {
            if (Baz.canIDoIt())
            {
                result = new Foo();
            }

            return result;
        }
        catch
        {
            if (result != null)
            {
                (result as IDisposable).Dispose();
                // IFoo does not inherit from IDisposable, hence the cast
            }

            throw;
        }
    }

    public static void Main()
    {
        IFoo bar = getDefaultFoo();
    }
}

Analysis report contain the folowing: `CA2000 : Microsoft.Reliability : In method 'DisposableFooTest.getDefaultFoo()', call System.IDisposable.Dispose on object 'result' before all references to it are out of scope. %%projectpath%%\DisposableFooTest.cs 44 Test

Edit2:

The following approach resolved CA2000 problem:

private static IFoo getDefaultFoo()
{
    Foo result = null;
    try
    {
        if (Baz.canIDoIt())
        {
            result = new Foo();
        }
        return result;
    }
    finally
    {
        if (result != null)
        {
            result.Dispose();
        }
    }
}

Unfortunately, I can't go that way. What is more, I'd rather expect following object-oriented principles, good practices and guidelines to simplify the code, make it readable, maintanable and extendible. I doubt anyone read it as intended: give the Foo if possible, or null otherwise.


This is a false positive warning. There is no way to return an appropriate instance of IFoo, if IFoo implements IDisposable, without the code analysis tool warning you that you're not disposing of it properly.

The code analysis doesn't analyze your intent or logic, it just tries to warn about common errors. In this case, it "looks like" you're using an IDisposable object and not calling Dispose(). Here, you're doing it by design, as you want your method to return a new instance and its acting as a form of factory method.

Returning a Disposable Object from a Method, So, in the second example the port. The disposable pattern is implemented with IDisposable interface to ensure mess was created by your attempted initialization instead of a clean, null copy, as in the MSDN example. IDisposable created within a method and returned (2) Static analysis is basically complaining for the following reason: The IFoo interface does not inherit from IDisposable, But you are returning a concrete implementation which must be disposed, and callers won't have a way of knowing this. In other words, the caller of your GetDefaultFoo method expects an IFoo implementation, but doesn't have a clue that your implementation needs to be explicitly disposed, and will therefore probably not


The problem here isn't something that your C# code is explicitly doing, but rather that the generated IL is using multiple instructions to accomplish what looks like one "step" in the C# code.

For example, in the third version (with the try/finally), the return retFoo line actually involves assignment of an additional variable that is generated by the compiler and that you cannot see in your original code. Since this assignment takes place outside the try/finally block, it really does introduce potential for an unhandled exception to leave your disposable Foo instance "orphaned".

Here's a version that will avoid the potential problem (and pass CA2000 screening):

private static IFoo getDefaultFoo()
{
    IFoo result = null;
    try
    {
        if (Baz.canIDoIt())
        {
            result = new Foo();
        }

        return result;
    }
    catch
    {
        if (result != null)
        {
            result.Dispose();
        }

        throw;
    }
}

Obviously, that hampers readability and maintainability quite a bit as compared to the first version, so you'll have to decide whether the likelihood of orphaning and the consequences of not disposing an orphan actually merit the code change, or whether suppression of the CA2000 violation might be preferable in this particular case.

"IDisposables" created in a "using" statement should not be returned, The exception to this rule is when your method returns that IDisposable . In that case using disposes of the object before the caller can make use of it, likely� A using block is appropriate with disposable objects in most cases, but does not address this situation where the object is being created within a method and returned from that method. The calling code will need to be responsible for the disposal of the object. It is therefore intentional that the port.Close remains open during normal execution.


Static analysis is basically complaining for the following reason:

  1. The IFoo interface does not inherit from IDisposable,
  2. But you are returning a concrete implementation which must be disposed, and callers won't have a way of knowing this.

In other words, the caller of your GetDefaultFoo method expects an IFoo implementation, but doesn't have a clue that your implementation needs to be explicitly disposed, and will therefore probably not dispose it. If this was a common practice in other libraries, you would have to manually check if an item implements IDisposable for, well, every possible implementation of any possible interface.

Obviously, there is something wrong with that.

IMHO, the cleanest way of fixing this would be to make IFoo inherit from IDisposable:

public interface IFoo : IDisposable
{
    void Bar();
}

This way, callers know that they any possible implementation they receive must be disposed. And this will also allow the static analyzer to check all places where you are using an IFoo object, and warn you if you are not disposing them properly.

C# Bug: Classes with "IDisposable" members should implement , The length returned from a stream read should be checked The class owns the field, and is therefore responsible for calling Dispose on it. is to call it in its own Dispose function, and therefore to be itself IDisposable . A class is considered to own an IDisposable field resource if it created the object referenced by the field. Rule 2: For a class owning managed resources, implement IDisposable (but not a finalizer). This implementation of IDisposable should only call Dispose() for each owned resource. It should not set anything to null. The class should not have a finalizer. Rule 3: For a class owning at least one unmanaged resource, implement both IDisposable and a


When returning an IDisposable object, in the event of a null value, dispose of the object and return null in the method - this should clear up the 'warning/recommendation' message associated with your 'private static IFoo getDefaultFoo()' method. Off course you still need to deal with the object as an IDisposable (using or dispose) in your calling routine.

private static IFoo getDefaultFoo()
{
    IFoo ret = null;
    if (Baz.canIDoIt())
    {
        retFoo = new Foo();
    }
    return ret;
}

Change to

private static IFoo getDefaultFoo()
{
    IFoo ret = null;
    if (Baz.canIDoIt())
    {
        retFoo = new Foo();
    }
    if (ret == null)
    {
        ret.dispose();
        return null;
    }
    else
        return ret;
}

Implement a Dispose method, Most APIs in .NET libraries that create an unmanaged resource will wrap it in a SafeHandle and return that SafeHandle to you as needed, rather� Implement a Dispose method. 05/27/2020; 15 minutes to read +10; In this article. Implementing the Dispose method is primarily for releasing unmanaged resources used by your code. . When working with instance members that are IDisposable implementations, it's common to cascade Dispose ca


CA2000: Dispose objects before losing scope, You've called a method on your object that calls Dispose , such as Close or wrapper that's created in the method and returned to the caller� The class implements the IDisposable interface and the required Dispose method. Note: In the Main method, we wrap the SystemResource instance inside a using statement. On execution: First the SystemResource instance is allocated upon the managed heap. Next the "1" is written. And: The "0" is written because the Dispose method is invoked. And


The curious case of async, await, and IDisposable, The second as an async method that returns the result of awaiting other work. These two methods look almost the same, but the code generated by the These two talks on InfoQ by Jon Skeet and I go into all the gory details� The method that raised the warning returns an IDisposable object that wraps your object The allocating method does not have dispose ownership; that is, the responsibility to dispose the object is transferred to another object or wrapper that's created in the method and returned to the caller


C# 8.0: Understanding Using Declarations, In this post, I introduce a C# 8 language feature called using today without using statements, by manually calling the Dispose method on such The Main method creates an instance of this type and calls the DoSomething method. Finally will run when the code in the try block returns and generally that� // If disposing equals true, the method has been called directly // or indirectly by a user's code. Managed and unmanaged resources // can be disposed. // If disposing equals false, the method has been called by the // runtime from inside the finalizer and you should not reference // other objects.