tags:

views:

92

answers:

4

If I pass an IDisposable object to a base class constructor using the base(...) construct, I trigger a silly error in FxCop about letting loose of an IDisposable. The warning is occasionally useful elsewhere, so I don't want to disable it, but I realize I don't know the exact semantics I should be using.

Is it the base constructor's responsibility to wrap its body in a try/catch to ensure the IDisposable thing is disposed of properly in case of exception?

Related questions:

+3  A: 

Not really an answer to the question, but...

You don't necessarily need to disable the rule altogether - you can suppress it for the methods where you know that the analysis is over-zealous:

[SuppressMessage("Microsoft.Reliability",
                 "CA2000:DisposeObjectsBeforeLosingScope",
                 Justification = "Your reasons go here")]
public YourClass(IDisposable obj) : base(obj)
{
}

Although the CA2000 analysis is so broken that it might be more useful to disable it altogether.

LukeH
+1  A: 

Yes, neither class constructor, nor using statement call Dispose in case of exception in constructor.

It's your job to handle exceptions in constructor, because object that isn't yet created can't be "disposed" in the same way as the successfully created one.

That's not really related to base constructor, it's a bigger problem:

class A: IDisposable
{
    public A() // constructor
    {
        r1 = new Resource(res1_id);     // resource aquisition 
        r2 = new Resource(res2_id);     // assume exception here
    }

    public void Dispose()
    {
        r1.Release();           // released successfully
        r2.Release();           // what to do?
    }

    Resource r1, r2;
}
Grozz
indeed, so it sounds like the base class needs to wrap its constructor function body with some try/catch logic so that the IDispoable thing it was asked to take custody of is properly disposed in the event of an exception in that body. Not many people follow that logic, but it is strictly correct.
Sebastian Good
+1  A: 

Simple rule to follow: if you create* it, it's your responsibility to see that it gets disposed. Conversely, if a method (be it a constructor or otherwise) does not create the disposable object, I would not expect for it to dispose of it. (*Creation includes invoking methods that return an IDisposable.)

So given something like

public Foo(SomeDisposable obj) : base(obj) { }

I would expect to see something like

using (SomeDisposable obj = new SomeDisposable())
{
    Foo foo = new Foo(obj);
}

And not

Foo foo = new Foo(new SomeDisposable());

If you have a parameter-less constructor for Foo and you want to call the base constructor with a disposable object, you're in a much trickier position, and that's probably where the rule is coming in trying to protect you. I would say avoid such a position by either coding the base to create and therefore be responsible for the IDisposable or have a 1:1 mapping between constuctors with IDisposable parameters, so that a parameterless Foo doesn't invoke a parameterized base.

But it shouldn't be the method or constructor's job to guess that you are done with the object you passed in, because how is it supposed to know? You could have other uses for it.

Anthony Pegram
A: 

I've come up with a nice pattern which I posted as an answer to my own related question here which allows one to safely combine the declaration and initialization of iDisposable fields and ensure they're disposed even when a constructor fails. The code could use a little cleaning up still, but it seems apropos here.

The one thing I can't figure out how to do is include event setup/cleanup as well. Having a list of objects which were allocated doesn't translate into a list of event subscriptions that need to be unwound, even via reflection.

supercat