views:

985

answers:

5

My observation in practice has been that GC.SuppressFinalize does not always suppress the call to the finalizer. It could be that the finalizer gets called nontheless. I wonder therefore whether GC.SuppressFinalize has the nature of a request rather than a guarantee by the system?


More Information

Following information may help provide more context for the quesiton if needed.

The GC.SuppressFinalize document summary does state that is a request:

Requests that the system not call the finalizer for the specified object.

I wonder if this was a casual use of the word or truly intended to describe the run-time behavior.

I have observed this with the following SingletonScope class taken from the Schnell project, which was based on an original idea by Ian Griffiths except it is more generalized. The idea is to detect, in debug builds, if the Dispose method did get called or not. If not, the finalizer would kick in eventually and one can put up a warning. If Dispose is called then GC.SuppressFinalize should prevent the finalizer from firing. Unfortunately, the warnings seem to fire anyhow, but not in a deterministic fashion. That is, they don't fire on each and every run.

#region License, Terms and Author(s)
//
// Schnell - Wiki widgets
// Copyright (c) 2007 Atif Aziz. All rights reserved.
//
//  Author(s):
//      Atif Aziz, http://www.raboof.com
//
// This library is free software; you can redistribute it and/or modify it 
// under the terms of the GNU Lesser General Public License as published by 
// the Free Software Foundation; either version 2.1 of the License, or (at 
// your option) any later version.
//
// This library is distributed in the hope that it will be useful, but WITHOUT
// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 
// FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public 
// License for more details.
//
// You should have received a copy of the GNU Lesser General Public License
// along with this library; if not, write to the Free Software Foundation, 
// Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA 
//
#endregion

namespace WikiPad
{
    #region Imports

    using System;
    using System.Diagnostics;

    #endregion

    //
    // NOTE: To use SingletonScope and ISingletonScopeHelper with value 
    // types, use Nullable<T>. For example, if the type of value to scope
    // is ThreadPriority then use ISingletonScopeHelper<ThreadPriority?>
    // and SingletonScope<ThreadPriority?>.
    //

    //
    // In debug builds, this type is defined as a class so a finalizer
    // can be used to detect an undisposed scope.
    //

    /// <summary>
    /// Designed to change a singleton and scope that change. After exiting
    /// the scope, the singleton is restored to its value prior to entering
    /// the scope.
    /// </summary>

    #if !DEBUG
    internal struct SingletonScope<T, H> 
    #else
    internal sealed class SingletonScope<T, H> 
    #endif
        : IDisposable 
        where H : ISingletonScopeHelper<T>, new()
    {
        private T _old;

        public SingletonScope(T temp)
        {
            _old = Helper.Install(temp);
        }

        private static H Helper
        {
            get { return new H(); }
        }

        public void Dispose()
        {
            //
            // First, transfer fields to stack then nuke the fields.
            //

            var old = _old;
            _old = default(T);

            //
            // Shazam! Restore the old value.
            //

            Helper.Restore(old);

            #if DEBUG
            GC.SuppressFinalize(this); // Only when defined as a class!
            #endif
        }

        #if DEBUG

        //
        // This finalizer is used to detect an undisposed scope. This will
        // only indicate that the scope was not disposed but (unfortunately)
        // not which one and where since GC will probably collect much later
        // than it should have been disposed.
        //

        ~SingletonScope()
        {
            Debug.Fail("Scope for " + typeof(T).FullName + " not disposed!");
        }

        #endif
    }
}


A full working example is available at http://gist.github.com/102424 with compilation instructions, but do note that the problem cannot reproduced deterministically so far.

A: 

I have used the exact same pattern many times and GC.SupressFinalize has always appeared to work.

Keep in mind that a call to GC.ReRegisterForFinalize will cause objects to re-register for finalisation.

Whenever I use the technique above I always ensure that include a full stack trace during object construction so I can track down the method that allocated the non-disposed object.

Eg. in the constructor use

StackFrame frame = new StackFrame(1);

and report that in your debug message during the finaliser.

Also, I notice your GC.SupressFinalize is not in a finally clause, if an exception is thrown during dispose, your objects finalizer will not be suppressed.

Sam Saffron
As you can see, there is no call to GC.ReRegisterForFinalize in the accompanying code.
Atif Aziz
I know that was just trying to ensure the answer is complete, did you try the stackframe debugging trick ?
Sam Saffron
@sambo99 I will consider adding that trick. Meanwhile, it won't help in this case because I know for sure Dispose was called because the scope object is used in conjunction with using from C#. I was wondering if I need to be more defensive in the finalizer.
Atif Aziz
@Atif, are you sure dispose did not throw an exception ?
Sam Saffron
also would it be possible for you to create a tiny sample that fails?
Sam Saffron
+2  A: 

One oddity you may be seeing is that the finalizer can still run even while an instance method is still running, so long as that instance method doesn't use any variables later on. So in your sample code, the Dispose method doesn't use any instance variables after the first line. The instance can then be finalized, even though Dispose is still running.

If you insert a call to GC.KeepAlive(this) at the end of the Dispose method, you may find the problem goes away.

Chris Brumme has a blog post about this, and I think there's another around somewhere...

Jon Skeet
Jon, I am well aware of that caveat, but the finalizer and Debug.Fail code in it seems to be firing when the application exist, which I suppose the pending finalizers are being executed. This happens *long* after GC.SuppressFinalize has been called.
Atif Aziz
@Jon some sample code that breaks Dispose would be enlightening, I wonder if a sample could be convoluted up.
Sam Saffron
@sambo99: I don't have time to write up a sample right now, but I'll try to do it later on. Basically if you make Dispose call GC.Collect()/GC.WaitForPendingFinalizers, you can see it finalize the instance before Dispose has finished.
Jon Skeet
@Atif: Hmm. Okay, that's weird. Do you have a complete example you could put up somewhere?
Jon Skeet
@Jon, @sambo99: I've now added a full working and self-contained sample that is available at http://gist.github.com/102424 but bear in mind that I have not been able to reproduce the conditions deterministically. In fact, and as is usually the case, I have not seen it occur so far with the isolated sample. :P Now, you'll tell me that probably my real application is buggy, right? ;) Nice try. Guess, you'll just have to trust me. :)
Atif Aziz
Well I'm not going to try to look at it in detail if you've *never* seen it in your self-contained example. If the self-contained example works, and the application doesn't, then it sounds like there's a significant difference between the two...
Jon Skeet
@Jon: I agree. I'm not expecting someone debug my case. I was just asking a simple question, which is whether it is known with absolute assurance that GC.SuppressFinalize is guaranteed or not. If someone says yes then ideally they can also provide a reference with some authority. Whether that sheds light on my issue or not remains my problem. :) The thing is that, docs say it's a request, not guarantee.
Atif Aziz
I don't know of anything that says it's a cast-iron guarantee, but I'd certainly suspect the application before the framework in this case. Removing the item from the finalizable queue is pretty straightforward - and while it's reasonable to be cautious just in case the finalizer thread has already decided what it needs to process before you call SuppressFinalize, it sounds like you don't really have that kind of race condition here, given what you've said about it being on application shut down.
Jon Skeet
Jon, I don't think this bug exists anymore in the clr http://gist.github.com/106190 try as hard as you will i don't think you can get it to fail
Sam Saffron
Instance methods should always report the object as a GC root while they are on the stack. In other words, `GC.KeepAlive(this)` should never serve any practical purpose.
280Z28
+1  A: 

I'm always using this design pattern to implement the IDisposable interface. (which is suggested by Microsoft) and for me GC.SuppressFinalize always has the nature of a guarantee!

using System;
using System.ComponentModel;

//The following example demonstrates how to use the GC.SuppressFinalize method in a resource class to prevent the clean-up code for the object from being called twice.

public class DisposeExample
{
    // A class that implements IDisposable.
    // By implementing IDisposable, you are announcing that 
    // instances of this type allocate scarce resources.
    public class MyResource : IDisposable
    {
        // Pointer to an external unmanaged resource.
        private IntPtr handle;
        // Other managed resource this class uses.
        private readonly Component component = new Component();
        // Track whether Dispose has been called.
        private bool disposed;

        // The class constructor.
        public MyResource(IntPtr handle)
        {
            this.handle = handle;
        }

        // Implement IDisposable.
        // Do not make this method virtual.
        // A derived class should not be able to override this method.
        public void Dispose()
        {
            Dispose(true);
            // This object will be cleaned up by the Dispose method.
            // Therefore, you should call GC.SupressFinalize to
            // take this object off the finalization queue 
            // and prevent finalization code for this object
            // from executing a second time.
            GC.SuppressFinalize(this);
        }

        // Dispose(bool disposing) executes in two distinct scenarios.
        // If disposing equals true, the method has been called directly
        // or indirectly by a user's code. Managed and unmanaged resources
        // can be disposed.
        // If disposing equals false, the method has been called by the 
        // runtime from inside the finalizer and you should not reference 
        // other objects. Only unmanaged resources can be disposed.
        private void Dispose(bool disposing)
        {
            // Check to see if Dispose has already been called.
            if (!disposed)
            {
                // If disposing equals true, dispose all managed 
                // and unmanaged resources.
                if (disposing)
                {
                    // Dispose managed resources.
                    component.Dispose();
                }

                // Call the appropriate methods to clean up 
                // unmanaged resources here.
                // If disposing is false, 
                // only the following code is executed.
                CloseHandle(handle);
                handle = IntPtr.Zero;
            }
            disposed = true;
        }

        // Use interop to call the method necessary  
        // to clean up the unmanaged resource.
        [System.Runtime.InteropServices.DllImport("Kernel32")]
        private extern static Boolean CloseHandle(IntPtr handle);

        // Use C# destructor syntax for finalization code.
        // This destructor will run only if the Dispose method 
        // does not get called.
        // It gives your base class the opportunity to finalize.
        // Do not provide destructors in types derived from this class.
        ~MyResource()
        {
            // Do not re-create Dispose clean-up code here.
            // Calling Dispose(false) is optimal in terms of
            // readability and maintainability.
            Dispose(false);
        }
    }

    public static void Main()
    {
        // Insert code here to create
        // and use a MyResource object.
    }
}

Source: MSDN: GC.SuppressFinalize Method

M. Jahedbozorgan
Bingo! Your implementation does not care whether the finalizer is called or not in the face of finalizer suppression because it is coded defensively. Your finalizer calls back into Dispose and has an extra flag to determine itself if it's been disposed or not. I placed a naked call to Debug.Fail in my finalizer assuming it will never be called if I use GC.SuppressFinalize, thus my question about the runtime's guarantees.
Atif Aziz
If you run my code, you will see that the destructor (finalizer) will run only if the Dispose method does not get called.
M. Jahedbozorgan
A: 

When an object with a user-defined finalizer is constructed, the runtime has to keep an internal reference to it so when it becomes unreachable in user code it can be still have the finalizer called in the runtime's finalization thread. Considering that time is of the essence when calling finalizers, it makes no sense to keep the objects in the queue if the user has requested them be suppressed. In my test CLI implementation, I keep a SuppressFinalizer flag in the header of objects that have user-defined finalizers. If the flag is true when the finalizer thread reaches that object in the queue, the finalizer call is skipped. I don't remove the object from the queue so I can keep the call to GC.SuppressFinalize() O(1) instead of O(N), where N is the number of allocated finalizable objects (I might change this policy to a deferred removal policy later).

280Z28
A: 

I throw an InvalidOperationException in the finalizer which makes it easy to find types which haven't been disposed properly. When Dispose() is called where GC.SuppressFinalize is invoked, I never get an exception.

codekaizen