object lifetime and garbage collection

Apr 14, 2009 at 10:53 PM
I have an object that gets created and put into the container as a singleton.
At some point, I no longer need the object and I tried to remove it.  From what I can see, there are no more references to the object but for some reason, it is not being garbage collected.
I am wondering if the container is keeping it alive.  Note that there is no external reference to the container either.  The container is created initially outside of the object and is passed into it as a constructor argument and the object holds onto the container as a member variable.  So we have a cyclic reference in a sense.

I've tried to call the container.Dispose but that only succeeded in "freezing" and eventually crashing my app.
Is there a way to properly shut down the container?  What else could be holding my reference hostage?

Thanks
Jiho
Apr 17, 2009 at 12:21 AM
Something has to be keeping the container alive. Perhaps using WinDBG/SoS could help you track down what's got your object rooted.

I have no idea why container.Dispose crashed your app - did you register the container with itself?
Apr 17, 2009 at 2:08 PM
Thanks for the suggestion - in fact, I have been using WinDBG/SoS to find out the culprit for the past couple of days.  I've never used the tool so it's taking me a while to get acquainted.
The Dispose issue is a known and resolved issue in Prism as it turns out.  Yes, Prism apparently had added the container itself to the container for some reason but no longer.
I've since fixed it in my local fork (I'm using the modified WinForms version).

I am using !gcroot command to find out what's keeping it afloat according to this article: http://blogs.msdn.com/ricom/archive/2004/12/10/279612.aspx
If you know other or better ways, please let me know.
Thanks
May 15, 2009 at 7:03 PM

Hi,

There is some issues witht the Dispose:

1.  the function does not call the    GC.SuppresFinalize(this);  and it definitely should

public override void Dispose()
{
    this.Dispose(true);
    GC.SuppresFinalize(this);  
}

2.  the class does not track if the dispose was allready called, creating a stackOverflow if the container itself is in the container (which is a common scenario I have seen)

private bool _diposed = defaut(bool);
private object lockObject = new object();
protected virtual void Dispose(bool disposing)
{
    lock(lockObject)
    {
    if (disposing)
    {
        if( !_disposed)
        {
           if (this.lifetimeContainer != null)
           {
               this.lifetimeContainer.Dispose();
               this.lifetimeContainer = null;
               if ((this.parent != null) && (this.parent.lifetimeContainer != null))
               {
                   this.parent.lifetimeContainer.Remove(this);
               }
           }
           foreach (IDisposable disposable in Sequence.OfType<IDisposable>(this.extensions))
           {
               disposable.Dispose();
           }
           this.extensions.Clear();
           _disposed = true;
        }
    }
   }
}

 Hope this makes it to the next release, StackOverflow are never nice to deal with !!!

--Robert

 

 

May 16, 2009 at 6:56 PM

Why should we call SuppressFinalize? The container doesn't have a finalizer, nor should it.

Your suggestion on the _disposed flag wouldn't help - if the container is registered with itself, it's stored in the lifetimeContainer, which is disposed before the flag gets set. Therefore the flag won't work. The real solution is NOT TO STORE THE CONTAINER IN ITSELF. You don't need to - in Unity 1.2 we already automatically register the container with itself in a way that will not cause a stack overflow. So Just Don't Do That.

 

-Chris

 

May 17, 2009 at 2:38 PM
Hi Chris,
Thanks for the precision. I definitely stop that practice right away!
However, I do believe that the compiler does generate a default finalizer if we don’t define one. Referring, please refer to this article : http://msdn.microsoft.com/en-us/magazine/cc163392.aspx . Before I did not use these recommendations and had all sorts for weird stuff happening at runtime and unreproducable situations... since I have been using this pattern, I no longer encounter (very rarely) these kind of problems or memory leaks.
Having this said, it does matter more on disposable object that have many instances. which is not really the case here since usually few containers exists in the same domaine space. However in the case where we are doing unit tests... I usually create a new container for each test.
I did not need to offend you or your team, I think that Unity is great. I discover new ways to use it everyday... Some stuff we have to discover, like the fact that the container is registered automatically.
--Bobby
May 17, 2009 at 8:44 PM

Bobby,

If my post sounded like I was offended, I apologize, as that was not my intent. The "I registered the container with itself" thing has come up many times now, and I just really wanted to emphasize that you don't have to do that anymore, it's done automatically. I'll try and get it documented better for Unity V2.

As far as the compiler generating a default finalizer, that I can definately answer: no, it doesn't. A finalizer is a very special method that changes the way the garbage collector works, and if every class had a finalizer, the overall performance of the GC (and the resulting system) would drop like a rock. The easiest way to check is to just write two classes, one with a finalizer and one without, and see what you get in ILDASM/Reflector. I wrote these two classes:

    class WithoutFinalizer
    {
        public void DoSomething()
        {
            Console.WriteLine("I'm doing something without a finalizer");
        }
    }

    class WithFinalizer
    {
        ~WithFinalizer()
        {
            Console.WriteLine("Finalizing");
        }

        public void DoSomething()
        {
            Console.WriteLine("I'm doing something with a finalizer");
        }
    }

When I crack open the assembly in Reflector, I see this for WithoutFinalizer (leaving out the method bodies):

.class private auto ansi beforefieldinit WithoutFinalizer
    extends [mscorlib]System.Object
{
    .method public hidebysig specialname rtspecialname instance void .ctor() cil managed
    {
    }

    .method public hidebysig instance void DoSomething() cil managed
    {
    }

}

And for WithFinalizer:

.class private auto ansi beforefieldinit WithFinalizer
extends [mscorlib]System.Object
{
.method public hidebysig specialname rtspecialname instance void .ctor() cil managed
{
}

.method public hidebysig instance void DoSomething() cil managed
{
}

.method family hidebysig virtual instance void Finalize() cil managed
{
}

}

So, you can see that the compiler only added the Finalize method to the class that explicitly defined it.

As for the Dispose pattern, I'd like to refer you to a paragraph near the bottom of that link you gave:

"

A strong argument can be made that with the introduction of SafeHandle, there is no longer any reason for a type not derived from SafeHandle to contain raw native resources. Instead, objects should only contain managed resources, including subclasses of SafeHandle for any native resources they use.

Following this pattern has several benefits. First, no classes other than the SafeHandles need to have finalizers, as only the SafeHandles will own native resources. Secondly, it creates a clean model where each SafeHandle owns exactly one native resource, and managed objects can then compose various SafeHandles to achieve their needs."

(Bold added by me).

This article was written in 2007. In the two years since, this argument (at least within Microsoft) has been settled - no classes other than SafeHandles should deal directly with unmanaged resources, and only SafeHandles should have finalizers. Since Unity doesn't deal with unmanaged resources at all, it has no reason to have a finalizer. And without a finalizer, there's no reason to complicate the Dispose implementation with the "dispose pattern". That pattern was very important in .NET 1.x, but it isn't nearly as important in the later CLRs.

Hope this helps,

-Chris

 

May 18, 2009 at 11:36 AM
Hi Chris,
Thanks for the extended explanation. Please be assured I will take this into consideration from now on.
Good day to you!
--Bobby

From: [email removed]
Sent: Sunday, May 17, 2009 3:44 PM
To: [email removed]
Subject: Re: object lifetime and garbage collection [unity:53281]

From: ctavares

Bobby,

If my post sounded like I was offended, I apologize, as that was not my intent. The "I registered the container with itself" thing has come up many times now, and I just really wanted to emphasize that you don't have to do that anymore, it's done automatically. I'll try and get it documented better for Unity V2.

As far as the compiler generating a default finalizer, that I can definately answer: no, it doesn't. A finalizer is a very special method that changes the way the garbage collector works, and if every class had a finalizer, the overall performance of the GC (and the resulting system) would drop like a rock. The easiest way to check is to just write two classes, one with a finalizer and one without, and see what you get in ILDASM/Reflector. I wrote these two classes:

class WithoutFinalizer
{
public void DoSomething()
{
Console.WriteLine("I'm doing something without a finalizer");
}
}

class WithFinalizer
{
~WithFinalizer()
{
Console.WriteLine("Finalizing");
}

public void DoSomething()
{
Console.WriteLine("I'm doing something with a finalizer");
}
}

When I crack open the assembly in Reflector, I see this for WithoutFinalizer (leaving out the method bodies):

.class private auto ansi beforefieldinit WithoutFinalizer
    extends [mscorlib]System.Object
{
    .method public hidebysig specialname rtspecialname instance void .ctor() cil managed
    {
    }

    .method public hidebysig instance void DoSomething() cil managed
    {
    }

}

And for WithFinalizer:

.class private auto ansi beforefieldinit WithFinalizer
extends [mscorlib]System.Object
{
.method public hidebysig specialname rtspecialname instance void .ctor() cil managed
{
}

.method public hidebysig instance void DoSomething() cil managed
{
}

.method family hidebysig virtual instance void Finalize() cil managed
{
}

}

So, you can see that the compiler only added the Finalize method to the class that explicitly defined it.

As for the Dispose pattern, I'd like to refer you to a paragraph near the bottom of that link you gave:

"

A strong argument can be made that with the introduction of SafeHandle, there is no longer any reason for a type not derived from SafeHandle to contain raw native resources. Instead, objects should only contain managed resources, including subclasses of SafeHandle for any native resources they use.

Following this pattern has several benefits. First, no classes other than the SafeHandles need to have finalizers, as only the SafeHandles will own native resources. Secondly, it creates a clean model where each SafeHandle owns exactly one native resource, and managed objects can then compose various SafeHandles to achieve their needs."

(Bold added by me).

This article was written in 2007. In the two years since, this argument (at least within Microsoft) has been settled - no classes other than SafeHandles should deal directly with unmanaged resources, and only SafeHandles should have finalizers. Since Unity doesn't deal with unmanaged resources at all, it has no reason to have a finalizer. And without a finalizer, there's no reason to complicate the Dispose implementation with the "dispose pattern". That pattern was very important in .NET 1.x, but it isn't nearly as important in the later CLRs.

Hope this helps,

-Chris

May 18, 2009 at 11:40 AM
Chris,
Regarding the container auto register itself, maybe it should throw an exception (mayby InvalidOperationException) when calling RegisterInsantce, and the object is itself.
--Bobby