DisposableWrappedObject.Dispose(disposing) seems unsafe to me

Mar 16, 2010 at 1:27 PM

Hello,

To me, this code is unsafe if called with disposing sets to false. According to the .NET specification, the Disposing object can already be disposed at the time we want to use it (should only access non managed objects).

       protected virtual void Dispose(bool disposing)
        {
            lock(this)
            {
                if(Disposed) return;

                EventHandler<GenericEventArgs<IDisposable>> handler = Disposing;
                if(handler != null)
                {
                    Disposing(this, new GenericEventArgs<IDisposable>(Instance));
                }

                Instance.Dispose();

                Disposed = true;
            }
        }
regards,
José
Coordinator
Mar 16, 2010 at 2:11 PM

Why is it unsafe?  the disposing member isn't even used in this implementation.  The reason the signature has it is in the event a desrived object wants to use it to fulfill a Disposed pattern.  It's also protected, so it's meant to be called only by subclasses.

Mar 16, 2010 at 3:31 PM

Say the Dispose() is not called (either directly or by utilizing the "using" keyword), then the GC will fire the code below:

        ~DisposableWrappedObject()
        {
            Dispose(false);
        }

and try to access the Disposing object in the Dispose method (which can be already GC).
Am I missing something?
Regards,
José
Coordinator
Mar 16, 2010 at 4:02 PM

If the finalizer runs, it calls Dispose.  If you haven't already called Dispose explicitly or via using, then Disposed will be false and the code will run.  The object cannot be GC'ed until that finalizer has run, so the objects are still valid (and the disposing variable itself is on the stack and not a candidate for GC).