locked
Finalizer implementation RRS feed

  • Question

  • Hello everyone,

     


    Two conflicting points about how to implement Finalizer, which one is correct?

     

    1.

     

    I always heard from people that in Finalizer we should not refer to any other objects, since the order of Finalizer is non-determistic, means the referred object in Finalizer might not alive any more.

     

    2.

     

    From the following article from MSDN, seems we are safe to refer to any wrapped objects (in my understanding, it means referring to member variables in Finalizer is always safe)?

    http://msdn2.microsoft.com/en-us/library/ms973837.aspx#dotnetgcbasics_topic2

    --------------------
    "How Finalization Affects Collection
    When the garbage collector first encounters an object that is otherwise dead but still needs to be finalized it must abandon its attempt to reclaim the space for that object at that time. The object is instead added to a list of objects needing finalization and, furthermore, the collector must then ensure that all of the pointers within the object remain valid until finalization is complete.
    ....
    Since the internal object pointers must remain valid, not only will the objects directly needing finalization linger in memory but everything the object refers to, directly and indirectly, will also remain in memory. If a huge tree of objects was anchored by a single object that required finalization, then the entire tree would linger, potentially for a long time as we just discussed."
    --------------------

     


    thanks in advance,
    George

     

    Sunday, April 20, 2008 7:20 AM

Answers

  • You are talking about two different things.  If I have a class with a finalizer and some member objects, then it is guaranteed that by the time of finalization my member objects will still be referenceable.  I.e., the member references will be accessible.

     

    However, if the member objects also have a finalizer, then it is possible for the member object's finalizer to get called before the wrapping object's finalizer.  So if from within your finalizer you use your member objects, it is possible that you are using them after they have been finalized, which could be the root of badness in the finalizer.

    Sunday, April 20, 2008 9:03 AM
  • That's right. You won't leak handles but you are in danger of losing buffered writes as Sasha states.

     

    You'd probably get away with calling Flush if you were just worried about writing buffered data to the underlying stream but I agree with Sasha that invoking Close is by far the best approach. Close invokes (or mimics) Dispose, as is usually the pattern for types of this nature.

     

    Note: However, you must not call StreamWriter.Close from within a finalizer. It should be invoked from Dispose logic.

    Monday, April 21, 2008 12:29 PM
  •  Sasha Goldshtein wrote:

    Assume you have a class and want to make sure that the developers using it will call Dispose.  You can implement the following pattern:

     

    Code Snippet

    class MyClass : IDisposable {

    public void Dispose() {

    //Dispose, call base.Dispose, etc. - cf. MSDN pattern

    GC.SuppressFinalize(this);

    }

     

    ~MyClass() {

    Debug.Assert(false, "MyClass finalizer called.  This means that an instance of MyClass was not Dispose'd of.  This can result in dire consequences ...");

    }

    }

     

     

     

     

    Re your question to Alex, indeed, you can't safely call StreamWriter.Close from your class' finalizer.  You'll have to implement Dispose.



    The above implementation can be used for sealed classes. For classes that can be descended from, use the following implementation instead

    Code Snippet

    public class MyClass : IDisposable
    {
        ~MyClass()
        {
            Dispose(false); // dispose of unmanaged resources only
        }

        public void Dispose()
        {
            Dispose(true); // dispose of managed + unmanaged resources
            GC.SuppressFinalize(this); // forget finalizer
        }

        protected virtual void Dispose(Boolean disposing)
        {
            if (disposing)
            {
                // dispose of managed resources here
            }

            // dispose of unmanaged resources here
        }
    }



    Tuesday, April 22, 2008 10:07 AM
  • Debug.Assert doesn't get called in a release build, and that's part of the idea.  The developer using your code will surely test it in the debug build, and see the assertion there.  However, if you want the assertion to appear in the release build, you can use Trace.Assert.

     

    Re question 2, what can happen is indeed that the FileStream's finalizer might be called before your finalizer, and then when you try to close the StreamWriter it will try closing a FileStream that has already been finalized.  Therefore it's not a safe thing to do from a finalizer, and you have no way of guaranteeing that the StreamWriter's buffer has been flushed.

     

    Tuesday, April 22, 2008 10:40 AM

All replies

  • You are talking about two different things.  If I have a class with a finalizer and some member objects, then it is guaranteed that by the time of finalization my member objects will still be referenceable.  I.e., the member references will be accessible.

     

    However, if the member objects also have a finalizer, then it is possible for the member object's finalizer to get called before the wrapping object's finalizer.  So if from within your finalizer you use your member objects, it is possible that you are using them after they have been finalized, which could be the root of badness in the finalizer.

    Sunday, April 20, 2008 9:03 AM
  • Thanks Sasha,

     

     

    Two more comments,

     

    1.

     

    If both wrapping object and wrapped object have Finalizer, the order of when they will be called are not determistic?

     

    2.

     

    How to write secure and reliable code in Finalizer for the wrapping object, in the situation when the wrapped object instance has Finalizer?

     

     Sasha Goldshtein wrote:

    You are talking about two different things.  If I have a class with a finalizer and some member objects, then it is guaranteed that by the time of finalization my member objects will still be referenceable.  I.e., the member references will be accessible.

     

    However, if the member objects also have a finalizer, then it is possible for the member object's finalizer to get called before the wrapping object's finalizer.  So if from within your finalizer you use your member objects, it is possible that you are using them after they have been finalized, which could be the root of badness in the finalizer.

     

     

    regards,

    George

    Sunday, April 20, 2008 10:14 AM
  • If both objects have a finalizer, the order of finalization is not deterministic.

    The only recommendation for the wrapping object's finalizer is to never assume that the member objects are in a consistent state.  Effectively it means that you shouldn't use them.
    Sunday, April 20, 2008 10:31 AM
  • Thanks Sasha,

     

     

    If the wrapping class has native resource, and it needs a Finalizer. At the same time, the wrapped component also contains native resource, and it also needs a Finalizer.

     

    In this situation, how do you write the Finalizer for the wrapping class? :-)

     

     Sasha Goldshtein wrote:
    If both objects have a finalizer, the order of finalization is not deterministic.

    The only recommendation for the wrapping object's finalizer is to never assume that the member objects are in a consistent state.  Effectively it means that you shouldn't use them.

     

     

    regards,

    George

    Sunday, April 20, 2008 11:28 AM
  • George,

     

    From the sounds of it you simply need to provide a finalizer for each class. Neither class should need to invoke the other during the finalization process. They should both be able to clean up after themselves autonomously.

    Sunday, April 20, 2008 12:22 PM
  • So both need a finalizer, and they needn't rely on each other to perform the finalization work.  If you could provide a specific scenario it would be easier to relate to see whether there is a problem or not.
    Sunday, April 20, 2008 2:57 PM
  • Thanks Sasha and Alex,

     

     

    Let me say in another way. :-)

     

    1.

     

    Suppose if in my class, there is a StreamWriter data member, in the Finalizer method, if I do not close the StreamWriter, there should not be any resource leak, right? (because the Finalizer of StreamWriter itself will clean up native file handle resource wrapped by StreamWriter?)

    2.

     

    So, C# I/O class like StreamWriter is managed resource, not unmanaged resource?

     

     Alex Cater wrote:

    George,

     

    From the sounds of it you simply need to provide a finalizer for each class. Neither class should need to invoke the other during the finalization process. They should both be able to clean up after themselves autonomously.

     


    regards,
    George

    Monday, April 21, 2008 11:34 AM
  • StreamWriter does not have a finalizer.  However, it wraps a FileStream which holds the file handle.  The FileStream class has a finalizer.  If you do not call StreamWriter.Close, you risk losing some of the data that is buffered within the StreamWriter.

     

    So you absolutely have to call StreamWriter.Close.

    Monday, April 21, 2008 12:00 PM
  • Thanks Sasha,

     

     

    I agree with your points from data integrity perspective.

     

    From resource leak perspective, we can rely on FileStream 's Finalizer to release native handle resource, right?

     

    (So, both my class and StreamWriter class do not need a file handle?)

     

     Sasha Goldshtein wrote:

    StreamWriter does not have a finalizer.  However, it wraps a FileStream which holds the file handle.  The FileStream class has a finalizer.  If you do not call StreamWriter.Close, you risk losing some of the data that is buffered within the StreamWriter.

     

    So you absolutely have to call StreamWriter.Close.

     

     

    regards,

    George

    Monday, April 21, 2008 12:25 PM
  • That's right. You won't leak handles but you are in danger of losing buffered writes as Sasha states.

     

    You'd probably get away with calling Flush if you were just worried about writing buffered data to the underlying stream but I agree with Sasha that invoking Close is by far the best approach. Close invokes (or mimics) Dispose, as is usually the pattern for types of this nature.

     

    Note: However, you must not call StreamWriter.Close from within a finalizer. It should be invoked from Dispose logic.

    Monday, April 21, 2008 12:29 PM
  • Alex is right.  You must provide a Dispose implementation and call StreamWriter.Close (or StreamWriter.Dispose) from there.  If you do that, you don't need to rely on FileStream's finalizer at all.  But in the worst case scenario where the user forgets to call your Dispose method, the file stream will be finalized but some data might not be flushed to the file.

     

    If you are very concerned with this last scenario, then you can always provide a finalizer implementation of your class and use Trace.Assert (or Debug.Assert) to alert the developer using your class of this scenario.

     

    Monday, April 21, 2008 12:38 PM
  • Thanks Alex,

     

     

    I recall of the question about what are safe to access in Finalizer method. I am not sure whether it is safe to access Close method of StreamWriter class, since I am not sure whether the Finalizer of wrapped StreamWriter member will be invoked before the Finalizer of current class is invoked? :-)

     

    Any comments?

     

     

     Alex Cater wrote:

    That's right. You won't leak handles but you are in danger of losing buffered writes as Sasha states.

     

    You'd probably get away with calling Flush if you were just worried about writing buffered data to the underlying stream but I agree with Sasha that invoking Close is by far the best approach. Close invokes (or mimics) Dispose, as is usually the pattern for types of this nature.

     

    Note: However, you must not call StreamWriter.Close from within a finalizer. It should be invoked from Dispose logic.

     

     

    regards,

    George

    Monday, April 21, 2008 12:44 PM
  • Thanks Sasha,

     

     

    I do not quite understand this trick -- "use Trace.Assert (or Debug.Assert) to alert the developer using your class of this scenario.". Could you show some pseudo code please? :-)

     

     Sasha Goldshtein wrote:

    Alex is right.  You must provide a Dispose implementation and call StreamWriter.Close (or StreamWriter.Dispose) from there.  If you do that, you don't need to rely on FileStream's finalizer at all.  But in the worst case scenario where the user forgets to call your Dispose method, the file stream will be finalized but some data might not be flushed to the file.

     

    If you are very concerned with this last scenario, then you can always provide a finalizer implementation of your class and use Trace.Assert (or Debug.Assert) to alert the developer using your class of this scenario.

     

     

     

    regards,

    George

    Monday, April 21, 2008 12:45 PM
  • Assume you have a class and want to make sure that the developers using it will call Dispose.  You can implement the following pattern:

     

    Code Snippet

    class MyClass : IDisposable {

    public void Dispose() {

    //Dispose, call base.Dispose, etc. - cf. MSDN pattern

    GC.SuppressFinalize(this);

    }

     

    ~MyClass() {

    Debug.Assert(false, "MyClass finalizer called.  This means that an instance of MyClass was not Dispose'd of.  This can result in dire consequences ...");

    }

    }

     

     

     

     

    Re your question to Alex, indeed, you can't safely call StreamWriter.Close from your class' finalizer.  You'll have to implement Dispose.

    Monday, April 21, 2008 2:05 PM
  • Thanks Sasha,

     

     

    1.

     

    Debug.Assert also works for a release version build?

     

    2.

     

    "you can't safely call StreamWriter.Close from your class' finalizer" means StreamWriter may be finalized before current class's Finalizer, so calling a method of Finalized instance is not safe? :-)

     

     Sasha Goldshtein wrote:

    Assume you have a class and want to make sure that the developers using it will call Dispose.  You can implement the following pattern:

     

    Code Snippet

    class MyClass : IDisposable {

    public void Dispose() {

    //Dispose, call base.Dispose, etc. - cf. MSDN pattern

    GC.SuppressFinalize(this);

    }

     

    ~MyClass() {

    Debug.Assert(false, "MyClass finalizer called.  This means that an instance of MyClass was not Dispose'd of.  This can result in dire consequences ...");

    }

    }

     

     

     

     

    Re your question to Alex, indeed, you can't safely call StreamWriter.Close from your class' finalizer.  You'll have to implement Dispose.

     

     

    regards,

    George

    Tuesday, April 22, 2008 9:13 AM
  • Hello, just an example I made ages ago.

     

    Code Snippet

    public abstract class Disposable : IDisposable
     {
      #region Instance Members
      private bool disposed = false;
      #endregion

      //////////////////////////////////////////////////////////////////////////////////////////

      #region IDisposable
      // Used to clean up your object resources
      public void Dispose() {
       this.Dispose(true);
       // Mr. GC this time we're taking care of the resources, stay away!
       GC.SuppressFinalize(this);
      }

      abstract protected void Dispose(bool disposing); /* {
       if(!this.disposed)
       {
        // Clean Unmanaged resources
        if(disposing)
        {
         // Clean Managed resources
        }
        this.disposed = true;
       }
      }*/
      #endregion

      // Use whenever you want to check whether the obejct was disposed or not
      protected bool Disposed {
       get { return this.disposed; }
       set { this.disposed = value; }
      }

      ~Disposable() {
       this.Dispose(false);
      }
     }

     

     

    Hope it helps.

     

    Tuesday, April 22, 2008 9:46 AM
  • Any comments or answers to my last questions, :-)

     

    1.

     

    Debug.Assert also works for a release version build?

     

    2.

     

    "you can't safely call StreamWriter.Close from your class' finalizer" means StreamWriter may be finalized before current class's Finalizer, so calling a method of Finalized instance is not safe? :-)

     

     Eyal-Shilony wrote:

    Hello, just an example I made ages ago.

     

    Code Snippet

    public abstract class Disposable : IDisposable
     {
      #region Instance Members
      private bool disposed = false;
      #endregion

      //////////////////////////////////////////////////////////////////////////////////////////

      #region IDisposable
      // Used to clean up your object resources
      public void Dispose() {
       this.Dispose(true);
       // Mr. GC this time we're taking care of the resources, stay away!
       GC.SuppressFinalize(this);
      }

      abstract protected void Dispose(bool disposing); /* {
       if(!this.disposed)
       {
        // Clean Unmanaged resources
        if(disposing)
        {
         // Clean Managed resources
        }
        this.disposed = true;
       }
      }*/
      #endregion

      // Use whenever you want to check whether the obejct was disposed or not
      protected bool Disposed {
       get { return this.disposed; }
       set { this.disposed = value; }
      }

      ~Disposable() {
       this.Dispose(false);
      }
     }

     

     

    Hope it helps.

     

     

     

    regards,

    George

    Tuesday, April 22, 2008 9:56 AM
  •  Sasha Goldshtein wrote:

    Assume you have a class and want to make sure that the developers using it will call Dispose.  You can implement the following pattern:

     

    Code Snippet

    class MyClass : IDisposable {

    public void Dispose() {

    //Dispose, call base.Dispose, etc. - cf. MSDN pattern

    GC.SuppressFinalize(this);

    }

     

    ~MyClass() {

    Debug.Assert(false, "MyClass finalizer called.  This means that an instance of MyClass was not Dispose'd of.  This can result in dire consequences ...");

    }

    }

     

     

     

     

    Re your question to Alex, indeed, you can't safely call StreamWriter.Close from your class' finalizer.  You'll have to implement Dispose.



    The above implementation can be used for sealed classes. For classes that can be descended from, use the following implementation instead

    Code Snippet

    public class MyClass : IDisposable
    {
        ~MyClass()
        {
            Dispose(false); // dispose of unmanaged resources only
        }

        public void Dispose()
        {
            Dispose(true); // dispose of managed + unmanaged resources
            GC.SuppressFinalize(this); // forget finalizer
        }

        protected virtual void Dispose(Boolean disposing)
        {
            if (disposing)
            {
                // dispose of managed resources here
            }

            // dispose of unmanaged resources here
        }
    }



    Tuesday, April 22, 2008 10:07 AM
  • Debug.Assert doesn't get called in a release build, and that's part of the idea.  The developer using your code will surely test it in the debug build, and see the assertion there.  However, if you want the assertion to appear in the release build, you can use Trace.Assert.

     

    Re question 2, what can happen is indeed that the FileStream's finalizer might be called before your finalizer, and then when you try to close the StreamWriter it will try closing a FileStream that has already been finalized.  Therefore it's not a safe thing to do from a finalizer, and you have no way of guaranteeing that the StreamWriter's buffer has been flushed.

     

    Tuesday, April 22, 2008 10:40 AM
  • Thanks Lasse,

     

     

    What is the key differences between your implementation and Sasha's implementation? I can not see too much differences. :-)

     

     Lasse Vågsæther Karlsen wrote:

     Sasha Goldshtein wrote:

    Assume you have a class and want to make sure that the developers using it will call Dispose.  You can implement the following pattern:

     

    Code Snippet

    class MyClass : IDisposable {

    public void Dispose() {

    //Dispose, call base.Dispose, etc. - cf. MSDN pattern

    GC.SuppressFinalize(this);

    }

     

    ~MyClass() {

    Debug.Assert(false, "MyClass finalizer called.  This means that an instance of MyClass was not Dispose'd of.  This can result in dire consequences ...");

    }

    }

     

     

     

     

    Re your question to Alex, indeed, you can't safely call StreamWriter.Close from your class' finalizer.  You'll have to implement Dispose.



    The above implementation can be used for sealed classes. For classes that can be descended from, use the following implementation instead

    Code Snippet

    public class MyClass : IDisposable
    {
        ~MyClass()
        {
            Dispose(false); // dispose of unmanaged resources only
        }

        public void Dispose()
        {
            Dispose(true); // dispose of managed + unmanaged resources
            GC.SuppressFinalize(this); // forget finalizer
        }

        protected virtual void Dispose(Boolean disposing)
        {
            if (disposing)
            {
                // dispose of managed resources here
            }

            // dispose of unmanaged resources here
        }
    }



     

     

    regards,

    George

    Tuesday, April 22, 2008 11:11 AM
  • Cool, Sasha!

     

     

    Your description is clear.

     

     Sasha Goldshtein wrote:

    Debug.Assert doesn't get called in a release build, and that's part of the idea.  The developer using your code will surely test it in the debug build, and see the assertion there.  However, if you want the assertion to appear in the release build, you can use Trace.Assert.

     

    Re question 2, what can happen is indeed that the FileStream's finalizer might be called before your finalizer, and then when you try to close the StreamWriter it will try closing a FileStream that has already been finalized.  Therefore it's not a safe thing to do from a finalizer, and you have no way of guaranteeing that the StreamWriter's buffer has been flushed.

     

     

     

    regards,

    George

    Tuesday, April 22, 2008 11:12 AM