IDisposable and CA2000 warning during Vs2010 Code Analysis.

[gumi_r msn com]

Hi all, I need some advice here, I hope somebody can help me. I have the following class structure (simplified):

public class Bar: IDisposable {...}

public abstract class FooBase: IDisposable
{
Bar bar;
bool disposed;

internal FooBase(Bar bar)
{
this.bar=bar;
}

public void Dispose()
{
if (!this.disposed)
{
this.bar.Dispose(true);
GC.SupressFinalize(this);
this.disposed = true;
}
}

protected void Dipose(bool disposing)
{
if (disposing)
{
this.bar.Dispose();
}
}
}

public FooA: Foo {...}
public FooB: Foo {...}

public static class FooProvider
{
public static FooA GetFooA()
{
Bar bar = new Bar();
...
return new FooA(bar);
}

public static FooB GetFooB()
{
Bar bar = new Bar();
...
return new FooB(bar);
}

...
}

When I run Code Analysis on this, I get Warnings CA2000 on all 'CreateFooX()' methods of the FooProvider class. This warning gives the following message: "Microsoft. Reliability: In method 'FooProvider.GetFooX()', call System.IDisposable.Dispose on object 'bar' before all references to it are out of scope." Microsoft recommends to never supress this warning but I'm not really sure its warning about a real problem in the code. True that 'bar' is not disposed before going out of scope in whatever 'CreateFooX()' method we consider but a reference to it lives on in the 'FooX' object which eventually will get disposed and will in turn take care of disposing 'bar'. Am I understanding something wrong about how the Dispose pattern should work and I have some fundamental flaw in my code or should I just supress this warning? Thanks for any advice.

[Subin Mavunkal]

you can fix this warning by applying 'using'.Do like this. public static FooA GetFooA() { using(var bar = new Bar()) { ... return new FooA(bar); } } public static FooB GetFooB() { using(var bar = new Bar()) { ... return new FooB(bar); } }

[Pete OHanlon]

OK. Now have a think about what effect calling Dispose on an item that you've just allocated via a constructor will be. Explain to the OP what happens when you attempt to access bar in FooB or FooA.

Forgive your enemies - it messes with their heads

My blog | My articles | MoXAML PowerToys | Mole 2010 - debugging made easier - my favourite utility

[gumi_r msn com]

Thanks for the reply but that is not a valid solution. If I do that, GetFooX() will be returning an invalid FooX object(disposed bar).

[Subin Mavunkal]

No , Just check the scope of "using". It will track the instance 'bar'

[Pete OHanlon]

This is why I said you should explain the effect of the using statement further, I knew it would cause you confusion.

Forgive your enemies - it messes with their heads

My blog | My articles | MoXAML PowerToys | Mole 2010 - debugging made easier - my favourite utility

[gumi_r msn com]

I'm sorry but that is wrong. A use statement is just syntactic sugar for a try finally block. What you are proposing translates to:

Bar bar = null;
try
{
bar = new Bar();
...
return new FooX(bar);
}
finally
{
if (bar != null)
{
bar.Dispose();
}
}

When the method gets to the return statement, it will execute the finally clause before exiting the method's scope returning a FooX object with a disposed bar. If you are not convinced execute the following code:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;

namespace Tests
{
class Program
{
static void Main(string[] args)
{
Console.WriteLine("Creating FooA 'fooA' from GetFooAWithUsingStatement.");
FooA fooA=FooProvider.GetFooAWithUsingStatement();
Console.WriteLine("'fooA' internal bar is disposed: {0}", fooA.InternalBarIsDisposed);
Console.WriteLine("Disposing 'fooA'");
fooA.Dispose();
Console.WriteLine("'fooA' internal bar is disposed: {0}", fooA.InternalBarIsDisposed);
Console.WriteLine();
Console.WriteLine("Creating FooA 'fooA' from GetFooAWithoutUsingStatement.");
fooA = FooProvider.GetFooWithoutUsingStatement();
Console.WriteLine("'fooA' internal bar is disposed: {0}", fooA.InternalBarIsDisposed);
Console.WriteLine("Disposing 'fooA'");
fooA.Dispose();
Console.WriteLine("'fooA' internal bar is disposed: {0}", fooA.InternalBarIsDisposed);
Console.Write("Press a key to exit.");
Console.ReadKey();
}
}

class Bar : IDisposable
{
    bool disposed;

    public void Dispose()
    {
        this.disposed = true;
    }

    public bool IsDisposed { get { return this.disposed; } }
}

static class FooProvider
{
    public static FooA GetFooAWithUsingStatement()
    {
        using (Bar bar = new Bar())
        {
            return new FooA(bar);
        }
    }

    public static FooA GetFooWithoutUsingStatement()
    {
        Bar bar = new Bar();
        return new FooA(bar);
    }
}

class FooBase : IDisposable
{
    bool disposed;
    Bar bar;

    internal FooBase(Bar bar)
    {
        this.bar = bar;
    }

    public void Dispose()

[Subin Mavunkal]

Then it is better to suppress the warning.

[DaveyM69]

IMO, the warning is invalid in this situation. If the method was simply returning Bar then you would not get the warning, so it's not understanding that returned Foo requires Bar so should not be disposed.

Dave
Binging is like googling, it just feels dirtier. Please take your VB.NET out of our nice case sensitive forum. Astonish us. Be exceptional. (Pete O'Hanlon)
BTW, in software, hope and pray is not a viable strategy. (Luc Pattyn)