Who Should Call Dispose on Idisposable Objects When Passed into Another Object

Who should call Dispose on IDisposable objects when passed into another object?

A general rule is that if you created (or acquired ownership of) the object then it is your responsibility to dispose it. This means that if you receive a disposable object as a parameter in a method or constructor you usually should not dispose it.

Note that some classes in the .NET framework do dispose objects that they received as parameters. For example disposing a StreamReader also disposes the underlying Stream.

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

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.

Does disposing an object dispose of all IDisposable properties?

Disposing an object doesn't automatically dispose all of its properties that are IDisposables. All it does is execute the Dispose method, what the object does then is up to the developer.

When an object of a class is disposed it should dispose all owned resources that implement the IDisposable interface. In your case you dispose an object that could very well be still in use.

I think the article from Stephen Cleary about IDisposable: What Your Mother Never Told You About Resource Deallocation explains about disposing objects and about the difficulties and problems that can arise in some circumstances.

Should I Dispose Object Passed Through Constructor If Other Constructors Create It?

You're correct in your thinking.

If you're passed an instance of Foo from elsewhere, and dispose of it yourself, you might break code still consuming the Foo. If you create an instance of Foo but don't dispose of it, you'll hang on to the memory it consumes unnecessarily long.

So, your best option is to track whether you created the instance yourself and dispose of it if so. (You could also allow the caller to specify the behaviour, as has been mentioned in the comments.)

(Alternatively, you could not do anything, which will be more inefficient but will at least not break anything. The only solution that is likely to actively break something is if you dispose of the instance that's been passed in from elsewhere.)

Should I dispose the IDisposable given to me?

You should not call Dispose in Load method, because you might do more with IDisposable object. using should use outside to dispose if you don't need more. Sample code:

using (var something = new Something())
{
    IFoo foo = new Foo();
    foo.Load(something);

    // Do more with something
}

Shared ownership of IDisposable objects in C#

I think the best you could do is something like this:

public sealed class SharedDisposable<T> where T : IDisposable
{
    public sealed class Reference : IDisposable
    {
        public Reference( SharedDisposable<T> owner )
        {
            mOwner = owner;
        }

        public void Dispose()
        {
            if( mIsDisposed ) return;
            mIsDisposed = true;

            mOwner.Release();
        }

        public T Value => mOwner.mValue;

        private readonly SharedDisposable<T> mOwner;
        private bool mIsDisposed;
    }

    public SharedDisposable( T value )
    {
        mValue = value;
    }

    public Reference Acquire()
    {
        lock( mLock )
        {
            if( mRefCount < 0 ) throw new ObjectDisposedException( typeof( T ).FullName );
            mRefCount++;
            return new Reference( this );
        }
    }

    private void Release()
    {
        lock( mLock )
        {
            mRefCount--;
            if( mRefCount <= 0 )
            {
                mValue.Dispose();
                mRefCount = -1;
            }
        }
    }

    private readonly T mValue;
    private readonly object mLock = new object();
    private int mRefCount;
}

Basically this allows you to have one object (SharedDisposable<T>) manage the lifetime of the underlying disposable while providing a mechanism to distribute "shared" references to it.

One shortcoming here is that technically anyone could dispose the underlying value by accessing it through the shared reference Value property. You could address this by creating some sort of facade object that wraps the underlying disposable type but hides its Dispose method.

How to properly dispose locally created object in another method?

will that object be disposed

Sorry, but that’s a meaningless question. The CLR does not keep track of whether an object has had its “dispose” method called or not (see Will the Garbage Collector call IDisposable.Dispose for me? )

As a general rule, it is always much nicer (readable/ maintainable / less-bug-prone / etc) that a method that creates an issue should also be the one that cleans up after itself. As you’ve just found, this pattern also allows automated checking by the compiler - and again, it is also a good rule to ensure that your code compiles cleanly without errors OR WARNINGS.

In this case, the warning is giving you a couple of ways to implement this cleanly; personally, I would prefer the “using” clause (so avoiding having to have an explicit call to “dispose”) like :

 public void SomeMethod()
 {
     using (DisposableObject disposableObject = new DisposableObject() )
      {

           // Do some stuff with the object

           SomeOtherMethod(disposableObject);
       }

}

Who Disposes of an IDisposable public property?

There is no single answer, it depends on your scenario, and the key point is ownership of the disposable resource represented by the property, as Jon Skeet points out.

It's sometimes helpful to look at examples from the .NET Framework. Here are three examples that behave differently:

• Container always disposes. System.IO.StreamReader exposes a disposable property BaseStream. It is considered to own the underlying stream, and disposing the StreamReader always disposes the underlying stream.

• Container never disposes. System.DirectoryServices.DirectoryEntry exposes a Parent property. It is not considered to own its parent, so disposing the DirectoryEntry never disposes its parent.

  In this case a new DirectoryEntry instance is returned each time the Parent property is dereferenced, and the caller is presumably expected to dispose it. Arguably this breaks the guidelines for properties, and perhaps there should be a GetParent() method instead.

• Container sometimes disposes. System.Data.SqlClient.SqlDataReader exposes a disposable Connection property, but the caller decides if the reader owns (and therefore disposes) the underlying connection using the CommandBehavior argument of SqlCommand.ExecuteReader.

Another interesting example is System.DirectoryServices.DirectorySearcher, which has a read/write disposable property SearchRoot. If this property is set from outside, then the underlying resource is assumed not to be owned, so isn't disposed by the container. If it's not set from outside, a reference is generated internally, and a flag is set to ensure it will be disposed. You can see this with Lutz Reflector.

You need to decide whether or not your container owns the resource, and make sure you document its behavior accurately.

If you do decide you own the resource, and the property is read/write, you need to make sure your setter disposes any reference it's replacing, e.g.:

public SomeDisposableObject SomeObject    
{        
    get { return m_someObject; }        
    set 
    { 
        if ((m_someObject != null) && 
            (!object.ReferenceEquals(m_someObject, value))
        {
            m_someObject.Dispose();
        }
        m_someObject = value; 
    }    
}
private SomeDisposableObject m_someObject;

UPDATE: GrahamS rightly points out in comments that it's better to test for m_someObject != value in the setter before disposing: I've updated the above example to take account of this (using ReferenceEquals rather than != to be explicit). Although in many real-world scenarios the existence of a setter might imply that the object is not owned by the container, and therefore won't be disposed.

Whether dispose is called automatically by GC when not explitly called?

Is in this case Dispose method will be called automatically by GC?

No. It will not call Dispose you have to call it yourself or use using block which will ensure its disposal even in case of an exception.

using(var disposable = new DisposableObject())
{
  //your code
}

IDisposable is used to ensure that unmanaged resources held by the application are released prior to garbage collection. Since GC can't release resources held by unmanaged resources like file handler.

Do i need to Dispose Stream when i Pass it to IDisposable class?

Yes, you can write code like that.

But, no, you should not do that.

Your class looks like on of XxxxReader classes that by convention own the stream they read from. As result your MyFileReader class is expected to dispose the inner stream. You also normally expected to dispose each and every disposable object when you know that such object's lifetime is over.

Note that sometimes it lead to multiple Dispose calls on some objects (which should be expected by implementations of IDisposable). While it may sometimes lead to Code Analysis warning it is better than missing Dispose calls if one routinely tries to "optimize" number of calls to Dispose by skipping some.

Alternative approach is to expose method that reads content which by convention is not expected to take ownership of stream/reader like:

 using(stream....)
 {
   var result = MyFileReader.ReadFrom(stream);
 }

---

Related Topics