Should we mark objects readonly if they are fields of an IDisposable class

Question

PROBLEM DESCRIPTION:

1. I have this class (FooClass) that implements IDisposable which takes another disposable (OtherDisposableClass) as an argument of its constructor.
2. I create an instance of OtherDisposableClass
3. I create an instance of FooClass and pass the instance OtherDisposableClass to it
4. The instance of OtherDisposableClass should not be mutateable so I mark it readonly
5. I can't get rid of the reference to the instance of OtherDisposableClass in my Dispose method

QUESTION

What is the best thing to do here? Should we

1. Not mark the instance of OtherDisposableClass as readonly so we can set it to 'null' in the Dispose method
2. Mark OtherDisposableClass as readonly and do not remove the reference to it ever again.
3. Something else

Please elaborate your answer. Thank you

public class FooClass : IDisposable
{
    private readonly OtherDisposableClass _disposable;
    private readonly string _imageSource;
    private readonly string _action;
    private readonly string _destination;

    private bool _isInitialized;

    public FooClass(OtherDisposableClass disposable)
    {
        _disposable = disposable;
    }
    ~FooClass() 
    {
        // Finalizer calls Dispose(false)
        Dispose(false);
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        // Not possible because _disposable is marked readonly
        _disposable = null;
    }
}

Answer 1

The thing that does the creation of OtherDisposableClass should be disposing of it... leave it alone in FooClass (you can have it readonly if you like).

Setting it to null will have no effect anyway, it will only set the reference from within FooClass to null (which is about to be disposed anyway).

EDIT

From the code given it looks like you don't even need to implement IDisposable here - this is just over complicating things. If you created an instance of OtherDisposableClass within the constructor of FooClass then you could implement IDisposable and simply call OtherDisposableClass.Dispose() within the FooClass.Dispose().

Here FooClass is created with a dependency on OtherFooClass - OtherFooClass will outlive FooClass - therefore FooClass should be GC'd first and then there will be no reference to OtherDisposableClass remaining - which will allow it to be GC'd

Answer 2

The IDisposable interface allows you to release unmanaged resources in a deterministic way. Unmanaged resources can be file handles, database connections etc. Disposing an object does not release any memory. Memory is only reclaimed by the garbage collector and you have very little control of when and how that is done. Disposing an object does not free any memory.

Still there is a connection between garbage collection and IDisposable. If your class directly controls an unmanaged resource it should release this resource during finalization. However, having a finalizer on a class has a performance impact and should be avoided if possible. FooClass does not directly control an unmanaged resource and the finalizer is not needed. However, to enforce the deterministic release of unmanaged resources it is important the FooClass.Dispose calls OtherDisposableClass.Dispose when Dispose is called explictly (when disposing is true). If Dispose is called from the finalizer you should not call methods on other managed objects because these objects may already have been finalized.

So to sum it up:

• Garbage collection and reclaiming of memory is distinct from releasing unmanaged resources.

• The Dispose method should always release unmanaged resource.

• The Dispose method should call Dispose on aggregated objects but only when called directly (not when called from the finalizer).

• It should be possible to call Dispose multiple times without any problems.

• There is no point in setting an IDisposable field to null in the Dispose method.

• Avoid implementing finalizers if you do not control any unmanaged resources directly.

• When implementing IDisposable you have to take inheritance into consideration because both the base class and the derived class may want to call Dispose on fields and/or release unmanaged resources. Sealing a class can avoid a lot of this complexity.

To elaborate on the point about setting the field to null:

I assume that your code is sane. E.g., only FooClass has a permanent reference to OtherDisposableClass and both objects will after disposal be unusable (e.g. throw ObjectDisposedException in all public methods except Dispose).

If you set the reference to OtherDisposableClass to null then that instance becomes eligible for garbage collection because there are no references to the instance. However, after disposing FooClass you surely will not keep an reference to that instance because it is unusable. This means that both the FooClass and the OtherDisposableClass are no longer reachable from a GC root and the entire object graph is now eligible for garbage collection. So there is no need to sever the link between FooClass and OtherDisposableClass because they will become eligible for garbage collection at the same time.