[Forward]Sweeping the IDisposable minefield

时间:2024-10-27 16:35:44
IDisposable has been around since the beginning of .Net.
The basic premise is simple..

Developers dont need to manage memory now. The 'Garbage Collector' takes care of reclaiming memory for you.. However the GC is non-deterministic.. you can't predict when it will embark on a 'collection' of unused objects. So far so good..
However there are cases when you want deterministic cleanup, e.g. if DB connections are a premium, you want to reclaim them as soon as possible. Enter IDisposable.. Simple concept. Introduce a public Dispose method that you can call when you deem fit.

Creating types

Summary:

  1. All managed objects do NOT need to implement IDisposable. Just let the GC do its thing. You do NOT need to implement IDisposable if
    1. does NOT directly own any unmanaged resources (e.g. native handles, memory, pipes, etc..)
    2. does NOT directly own any managed members that implement IDisposable
    3. does NOT have special cleanup needs that must run on-demand/asap e.g. unsubscribe from notifiers, close DB Connections, etc.
  2. If you need to implement IDisposable BUT do not own any (direct) unmanaged resources, You should NOT throw in a free finalizer.
    1. Consider if you can make your type sealed. This simplifies the implementation a great deal.
    2. If you must have subtypes, then implement the version with the virtual Dispose(bool) overload as detailed below. Again, rethink if you can seal the type.
  3. If you directly own unmanaged resources that need cleanup, check for a managed wrapper type that you can use. e.g. a SafeFileHandle. If there is, use it and fall back to 2. Still no finalizer
  4. If you reach here, you need a finalizer. Finalizer pulls in IDisposable. Ensure that you have a deterministic Dispose implementation, that makes the finalizer redundant and avoids the associated performance penalties. Log an error in the finalizer to call your attention to cases where the clients have forgotten to call Dispose. Fix them.
    1. Consider creating a managed wrapper type because finalizers are hard to get right. e.g. SafeMyNativeTypeWrapper. Deriving from SafeHandle is not recommended - better left to experts
    2. Use GC.AddMemoryPressure and its counterpart to 'help' the GC if you are allocating significant amounts of native memory. Similarly manage handles via the HandleCollector class (e.g. GDI handles). See this post for details except I'd move the Remove.. calls into Dispose instead of the finalizer.

Programming against types that implement IDisposable

  1. Limit the scope of disposable types to within a method. Wrap them within a using blockto ensure that Dispose is called (reliably) when control leaves the using block.
  2. If you need to hold on to a disposable type i.e. as a member field, you need to implement IDisposable on the container type. e.g. If A owns B owns C owns D, where D implements IDisposable, then A,B and C need to implement IDisposable as well.
  3. Do not dispose objects that you don't own. e.g. if you obtain a reference to an object from a container (e.g. a MEF container or a Form's controls collection) OR a static/global accessor  you don't own the object. Hence you shouldn't call dispose and break other clients with ObjectDisposedException. Leave it to the container to Dispose it.

The long-winded version (with code snippets)

Para1: Avoid implementing IDisposable unless necessary, most objects don't need it.

If your type doesn't need IDispose. You can stop reading here.

Para2: If you need deterministic cleanup, implement IDisposable (mini).

  • All public members need to check for _isDisposed == true & throw an ObjectDisposedException
  • Dispose can be called multiple times : once again use _isDisposed and ignore all calls except the first one
  • Dispose should not throw exceptions
  • Call Dispose for disposable managed objects that this type owns. Corollary: Dispose will creep all the way up the object graph. e.g. If TypeA contains B contains C contains D and D implements IDisposable: A,B,C need to implement IDisposable.
  • (Managed Memory Leaks from Events) - Unsubscribe from all events that this object has active subscriptions to. Long-lived Publishers can keep short-lived subscribers alive and prevent them from being collected. Try: clearing subscribers from your own events might be a good idea- set event/delegate to null. Try: using the WeakEventManager/WeakReference type
  • Seal your type - inheritance needs the full blown Dispose pattern (later on in this post).
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
sealed class MyType : IDisposable
    {
        // other code
        private bool _isDisposed;
        public void SomeMethod()
        {
            if (_isDisposed)
                throw new ObjectDisposedException();
            // proceed..
        }
        public void Dispose()
        {
            if (_isDisposed)
                return;
            // cleanup
            _isDisposed = true;
        }
    }

Para3: Avoid finalizers unless necessary.

When are finalizers necessary ?

  • When you have directly owned Unmanaged resources that need to be cleaned up AND there isn't a managed wrapper type that has the finalization routine nailed down e.g. a SafeHandle derivation. If you can, you go back to the previous section.

Finalizers

  • slow down the collection process
  • prolong object lifetime - the object moves into the next generation (whose collection is even less frequent. C# in a Nutshell gives a ratio of Gen 0 10:1  Gen 1)
  • are difficult to get right

Finalizers should

  • not block / throw exceptions
  • must execute quickly
  • not reference other finalizable members (their finalizers may have already run)
  • should log / raise a red flag to indicate Dispose was not called.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
sealed class HasUnmanaged : IDisposable
    {
        public void Dispose()
        {
            Dispose(true);
            // prevent object from being promoted to next Gen/finalizer call
            GC.SuppressFinalize(this);
        }
        ~HasUnmanaged()
        {
            LogSomeoneForgotToCallDispose();
            Dispose(false);
        }
        private bool _isDisposed;
        private void Dispose(bool isDisposing)
        {
            if (_isDisposed)
                return;
            if (isDisposing)
            {
                // dispose of managed resources(can access managed members)
            }
            // release unmanaged resources
            _isDisposed = true;
        }
    }

Para4: Subclassing a Disposable type

If your type cannot be sealed, then it's time to bring out the big guns. Implement the base type as follows

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
class BaseType : IDisposable
    {
        Intptr _unmanagedRes = ... // unmanaged resource
        SafeHandle _managedRes = ... // managed resource
    
        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }
        
        ~BaseType()
        {
            Dispose(false);
        }
    
        private bool _isDisposed;
        virtual void Dispose(bool isDisposing)
        {
            if (_isDisposed)
                return;
            if (isDisposing)
            {
                // managed resources dispose
                _managedRes.Dispose();
            }
            // unmanaged resource cleanup
            Cleanup(_unmanagedRes);
            // null out big fields if any
            _unmanagedRes = null;
            _isDisposed = true;
        }
    }
  • If the derived type has it's own resources that need cleanup, it overrides the virtual member like this
  • If the derived type does not have resources that need cleanup, just inherit from the base. You're done.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
class MessyDerived : BaseType
    {
        // ... Derived members
        private bool _isDisposed;
        override void Dispose(bool isDisposing)
        {
            try
            {
                if (!_isDisposed)
                {
                    if (isDisposing)
                    {
                        // derived managed resources
                    }
                    // derived unmanaged resources
                    _isDisposed = true;
                }
            }
            finally
            {
                base.Dispose(isDisposing);
            }
        }
    }
    class SimpleDerived : BaseType
    {
        // ... Derived members
    }

Of course, there will be edge-cases. But for the most part, this should save you a lot of grief
See also - Joe Duffy's post