What is the resulting behavior when an IDisposable is passed into a parent IDisposable

Question

Yesterday, after running Visual Studio code analysis on our codebase, the following code was highlighted as an issue:

using (var stringReader = new StringReader(someString))
{
    using (var reader = XmlReader.Create(stringReader)) {
    // Code
    }
}

The warning that was returned was

Warning CA2202 Object 'stringReader' can be disposed more than once in method '(method name)'. To avoid generating a System.ObjectDisposedException you should not call Dispose more than one time on an object.

After searching stack overflow, I've come to the general understanding that if I were to create a custom class containing IDisposable members, it should implement IDisposable itself, and call the dispose() method of the member.

My two questions are

• In all cases where object X takes a reference to an IDisposable object Y as a parameter during creation, is it correct to assume that the object X will take ownership of Y and from that point onwards, calling X.dispose() will always result in calling Y.dispose()
• This is an old piece of code and exceptions as described in the warning message have never been reported (to the best of my knowledge). If the above point is assumed, why does the double using block not result in calling stringReader.dispose() twice and therefore throwing an exception?

Answer 1

is it correct to assume that the object X will take ownership of Y and from that point onwards, calling X.dispose() will always result in calling Y.dispose()

No, it is never save to assume that. Let's check this specific case: XmlReader.Create(Stream).

After going to quite some code in the Reference Source, I have found that the Dispose method calls the Close method. That is quite obvious. Then notice this piece of code:

public override void Close() {
    Close( closeInput );
}

So whether the backing stream will be closed and disposed depends on the value of the setting closeInput, which you can set through the XmlReaderSettings.CloseInput setting.

So the answer here is a definite no: you can't be sure it is disposed. You should always make sure yourself it is.

Answer 2

• No, you cannot assume that another object will call Dispose() while disposing itself. The object having the disposable object as a reference might not even be using the disposable resource.
• This warning is known to be somewhat weird. Look here to see some complaints about the warning. You should design your class so that it must be safe to call Dispose() more than once.

By the way, MSDN says:

A method implementation contains code paths that could cause multiple calls to IDisposable.Dispose or a Dispose equivalent, such as a Close() method on some types, on the same object.

So a path to the Close() method call can generate this warning too, which is why you see this warning in your case.

Answer 3

In all cases where object X takes a reference to an IDisposable object Y as a parameter during creation, is it correct to assume that the object X will take ownership of Y and from that point onwards, calling X.dispose() will always result in calling Y.dispose()

I think not and I will try to explain why.

There is a pattern called IDisposablePattern which looks something like that:

public class SimpleClass : IDisposable
{
    // managed resources SqlConnection implements IDisposable as well.
    private SqlConnection _connection;
    private bool _disposed;

    // implementing IDisposable
    public void Dispose()
    {
        // Here in original Dispose method we call protected method with parameter true,
        // saying that this object is being disposed.
        this.Dispose(true);

        // Then we "tell" garbage collector to suppress finalizer for this object because we are releasing
        // its memory and doesnt need to be finalized. Calling finalizer(destructor) of a given type is expensive
        // and tweaks like this help us improve performance of the application.
        GC.SuppressFinalize(this);
    }

    // Following the best practices we should create another method in the class 
    // with parameter saying whether or not the object is being disposed.
    // Its really important that this method DOES NOT throw exceptions thus allowing to be called multiple times 
    protected virtual void Dispose(bool disposing)
    {
        // another thing we may add is flag that tells us if object is disposed already
        // and use it here
        if (_disposed) { return; }
        if (_connection != null)
        {
            _connection.Dispose();
            _connection = null;
        }
        _disposed = true;

        // call base Dispose(flag) method if we are using hierarchy.
    }
}

Note that this can be extended to new level when your class uses unmanaged resources like this one:

    public class SimpleClass2: IDisposable
{
    // managed resources
    private SqlConnection _connection;
    private bool _disposed;

    // unmanaged resources
    private IntPtr _unmanagedResources;

    // simple method for the demo
    public string GetDate()
    {
        // One good practice that .NET Framework implies is that when object is being disposed
        // trying to work with its resources should throw ObjectDisposedException so..
        if(_disposed) { throw new ObjectDisposedException(this.GetType().Name);}

        if (_connection == null)
        {
            _connection = new SqlConnection("Server=.\\SQLEXPRESS;Database=master;Integrated Security=SSPI;App=IDisposablePattern");
            _connection.Open();
        }
        // allocation of unmanaged resources for the sake of demo.
        if (_unmanagedResources == IntPtr.Zero)
        {
            _unmanagedResources = Marshal.AllocHGlobal(100 * 1024 * 1024);
        }

        using (var command = _connection.CreateCommand())
        {
            command.CommandText = "SELECT getdate()";
            return command.ExecuteScalar().ToString();
        }
    }


    public void Dispose()
    {
        // Here in original Dispose method we call protected method with parameter true,
        // saying that this object is being disposed.
        this.Dispose(true);

        // Then we "tell" garbage collector to suppress finalizer for this object because we are releasing
        // its memory and doesnt need to be finalized. Calling finalizer(destructor) of a given type is expensive
        // and tweaks like this help us improve performance of the application.

        // This is only when your class doesnt have unmanaged resources!!!
        // Since this is just made to be a demo I will leave it there, but this contradicts with our defined finalizer.
        GC.SuppressFinalize(this);
    }

    // Following the best practices we should create another method in the class 
    // with parameter saying wether or not the object is being disposed.
    // Its really important that this method DOES NOT throw exceptions thus allowing to be called multiple times 
    protected virtual void Dispose(bool disposing)
    {
        // another thing we may add is flag that tells us if object is disposed already
        // and use it here
        if (_disposed) { return; }
        // Thus Dispose method CAN NOT release UNMANAGED resources such as IntPtr structure,
        // flag is also helping us know whether we are disposing managed or unmanaged resources
        if (disposing)
        {
            if (_connection != null)
            {
                _connection.Dispose();
                _connection = null;
            }
            _disposed = true;
        }
        // Why do we need to do that?
        // If consumer of this class forgets to call its Dispose method ( simply by not using the object in "using" statement
        // Nevertheless garbage collector will fire eventually and it will invoke Dispose method whats the problem with that is if we didn't 
        // have the following code unmanaged resources wouldnt be disposed , because as we know GC cant release unmanaged code.
        // So thats why we need destructor(finalizer).
        if (_unmanagedResources != IntPtr.Zero)
        {
            Marshal.FreeHGlobal(_unmanagedResources);
            _unmanagedResources = IntPtr.Zero;;
        }
        // call base Dispose(flag) method if we are using hierarchy.
    }

    ~DatabaseStateImpr()
    {
        // At this point GC called our finalizer method , meaning 
        // that we don't know what state our managed resources are (collected or not) because
        // our consumer may not used our object properly(not in using statement) so thats why
        // we skip unmanaged resources as they may have been finalized themselves and we cant guarantee that we can
        // access them - Remember? No exceptions in Dispose methods.
        Dispose(false);
    }
}