Skip to main content
GameDev.net gamedev.net
🔒 Locked

GC with memory leaks?

Started by SnprBoB86 May 10, 2002 at 6:06 PM 11 replies 4.1k views
Original Post
SnprBoB86
SnprBoB86
When garbage collection causes memory leaks... I''m sure that having read the subject of this thread you are thinking: "I thought garbage collection existed to prevent memory leaks!" Well it does, unfortunatly it''s causing me quite the head ach right now because some objects are not being released because of a trickly little referance to it which wont go away. Anyone with any experiance in a garbage collected environment such as Java or C#, please read on. Basicly, I am using C# to implement a game engine. This game engine will be using a fancy event system for notification of occurances in the game world. This sounds cool and seems easy with the event handling functionality that .NET provides, but it has introduced a major memory leak where the memory is only realeased when the game shutsdown. The games evironment class contains a delegate (function pointer that works on instances of classes) that is multicast (can point to multiple functions). When an event occurs, all the functioned referanced by the delegate are called, notifiying the classes which requested the notification. Heres a quick, trimmed down snippit from my C# code:
  
delegate void VoidDelegate();

class Environment
{
     public Update()
     {
         // some explosion occurs

          if(Explosion != null)
               Explosion();
     }
     public VoidDelegate Explosion;
}

class NPC
{
     public NPC(Environment env)
     {
          env.Explosion += new VoidDelegate(this.ReactToExplosion);
     }
     protected void ReactToExplosion()
     {}
}
  
Now lets say an NPC is instanced. It''s constructor requests a notification from the environment. Then our NPC dies. Normally he would be released from memory, however he is not because the garbage collector detects a referance to him still exists. Where? The Explosion VoidDelegate inside the Environment!! The only solution I see is to remove all referances from multicast delegate by calling a function in NPC. Let''s see what this would look like.
  
class NPC
{
     public NPC(Environment env)
     {
          env.Explosion += new VoidDelegate(this.ReactToExplosion);
     }
     protected void ReactToExplosion()
     {}
     public RemoveNotifications(Environment env)
     {
          env.Explosion -= new VoidDelegate(this.ReactToExplosion);
     }
}
  
Can anyone think of a better solution??
Epolevne
Epolevne
So the behavior you''re seeing is that "subscribing" to an event is seen by the GC as a reference to the object in the active thread?

I suppose it would make sense, if the object is attaching to events then the object should remove its handlers.

Also, get used to having a Dispose() method on your objects to do immediate cleanup of all your precious resources (db connections, files, event handles apparently) so you don''t have to wait on the GC.
Just have your NPC keep a reference to the environment and remove the handles in the Dispose method.

Of course this all assumes that that''s the reason for the issue you''re seeing. I don''t really feel like making a simple mock-up right now, but the extra, "hidden" active reference seems reasonable.

Epolevne
Sieggy
Sieggy
As you probably already figured out their is nothing magical about garbage collectors in C# and Java. If you hold on to a reference somewhere you effectively have a leak. If you never release it or it doesn''t go out of scope there is no way for the gc to determine whether you need it or not. It will simply go by whether or not it can get to your reference through your current objects in memory. This is something to remember since there certainly is a conception that in Java and C# one never has to worry about memory again. Completely wrong! Not to mention the leaks that you will have are typically ones that are the hardest the track.
Kylotan
Kylotan
No, there''s no better solution. When you call the function to make an NPC die, that NPC needs to let everything that holds a reference to it know that it''s gone. If you register the object with some central source, you must deregister it. If the NPC references some other object, you must release that reference. And so on. Garbage collection actually doesn''t save you all that much effort in terms of resource acquisition and release - it mainly just isolates you from having to worry about the allocation of physical memory.

[ MSVC Fixes | STL | SDL | Game AI | Sockets | C++ Faq Lite | Boost | Asking Questions | Organising code files ]
Epolevne
Epolevne
Kylotan-
That''s not exactly true...you don''t have to release all your references for an object to be GC''d. Just your references in the active threads.

So if your NPC class has a reference to 100 other objects and it''s the only class that has these references, then when the NPC object is marked for collection so are all 100 member objects. Even if the member objects have back-references to the parent...since neither reference (parent-child, child-parent) is in an active thread all objects are considered "dead" and are collected.

The issue he''s having is an odd one...where a delgate passes a "this" reference...and since the Environment object that he''s attaching to will always be in the active thread (until the program is over) the objects are never GC''d.


Epolevne
SnprBoB86
SnprBoB86
Epolevne-

I put some code into the finalizers to notify me when the objects were released from memory and then ran some various tests. The tests indeed concluded that the multicast delegate was preventing the object from being released from memory.

Sieggy-

Yea, garbage collection is a bit tricky some times. Helpful and praised until some referance goes by unnoticed and the memory starts to fill up :-(

Kylotan-

I believe that OrangyTang has hit apon the solution...

OrangyTang-

I am now looking into soft referances and using them for event handling. I will post my findings.

Epolevne (again)-

You hit the nail on the head. It is definitly a odd problem, but there must be some design pattern to deal with a situation like this. I can not imaging that this problem hasn''t come up in other garbage collected environments in the past.

-SniperBoB-
Shannon Barber
Shannon Barber
If the object is staying in existence because of a reference count, it''s not a GC problem - it''s a referencing counting problem - which employees explicit delete''s and doesn''t relie on a GC''tor. If it''s a circular reference problem, then you need to make one of them "weak" (as in it either never calls the AddRef method, or calls the Release method upon acquiring the interface) as OrangyTang mentioned.

If the GC isn''t deallocating it when it could, it''s not exactly a leak, it''s more like a pool. One of two classic GC "issues".

Magmai Kai Holmlor

"Oh, like you''ve never written buggy code" - Lee

[Look for information | GDNet Start Here | GDNet Search Tool | GDNet FAQ | MSDN RTF[L] | SGI STL Docs | STFW | Asking Smart Questions ]

[Free C++ Libraries | Boost | ACE | Loki | MTL | Blitz++ | wxWindows]

Shamelessly ripped from Oluseyi
The trade-off between price and quality does not exist in Japan. Rather, the idea that high quality brings on cost reduction is widely accepted.-- Tajima & Matsubara
SnprBoB86
SnprBoB86
quote:
Use System.WeakReference, then cast it to the delegate class when you need to use it.


Thats brilliant! If I maintain an array of WeakReferances and then write methods to add and remove notifications I can easily create a WeakMulticastDelegate class!

Thanks so much, I will get right to coding this class and post it up for everyone elses benifit.

-SniperBoB-
SnprBoB86
SnprBoB86

  
public class WeakDelegate
{
public WeakDelegate(Delegate d)
{
referance = new WeakReference(d);
}

public object Invoke(object[] args)
{
return ((Delegate referance.Target).DynamicInvoke(args);
}

private WeakReference referance;
}


now on to a WeakMulticastDelegate !

[edited by - snprbob86 on May 12, 2002 12:10:51 PM]
SnprBoB86
SnprBoB86
WARNING - that class doenst work

after writing a WeakMulticastDelegate and doing some testing i found a MAJOR PROBLEM

the problem is that the weak referance is to the delegate, so if the delegate dies then the WeakDelegate doesnt work even though the class the delegate refers to is still alive :-(

i''m working on a fix

-SniperBoB-
SnprBoB86
SnprBoB86
FINAL MESSAGE

I have solved everything and even tested my classes versus standard multicast delegates. My WeakMulticastDelegate (source below) is only a thousandth of a second slower than a standard MulticastDelegate on 1,000 method calls!!


  
using System;
using System.Reflection;
using System.Collections;

namespace /*NAMESPACE NAME HERE*/
{
/// <summary>Delegate that will not prevent garbage collection</summary>

public class WeakDelegate
{
/// <summary>Construct a WeakDelegate</summary>

/// <param name="d">"Strong" Delegate to copy from</param>

public WeakDelegate(Delegate d)
{
referance = new WeakReference(d.Target);
methodName = d.Method.Name;
}

/// <summary>Construct a WeakDelegate</summary>

/// <param name="o">Object to invoke method on</param>

/// <param name="m">Method to invoke</param>

public WeakDelegate(object obj, string methodName)
{
this.referance = new System.WeakReference(obj);
this.methodName = methodName;
}

/// <summary>Call the method referanced by this WeakDelegate</summary>

/// <param name="args">Arguments to pass called method</param>

/// <returns>Results of method call</returns>

public object Invoke(object[] args)
{
return referance.Target.GetType().InvokeMember(methodName, BindingFlags.InvokeMethod, null, referance.Target, args);
}

/// <summary>Determins if the object to invoke the method on is alive</summary>

public bool IsAlive
{ get { return referance.IsAlive; } }

/// <summary>Compare WeakDelegates for equality</summary>

/// <param name="obj">Compare this to "obj"</param>

/// <returns>True if WeakDelegates are equivialent</returns>

public override bool Equals(object obj)
{
if(obj == null || GetType() != obj.GetType())
return false;
WeakDelegate d = (WeakDelegate)obj;
return ((this.referance.Target == d.referance.Target) && (this.methodName == d.methodName));
}

/// <summary>Get the hash code of this WeakReferance</summary>

/// <returns>Hash code</returns>

public override int GetHashCode()
{
return methodName.GetHashCode() + referance.Target.GetHashCode();
}

private string methodName;
private WeakReference referance;
}

/// <summary>Multicast Delegate which does not prevent garbage collection</summary>

public class WeakMulticastDelegate
{
/// <summary>Call all methods referanced by this WeakMulticastDelegate.

/// Also removes any delegates that point to dead objects</summary>

/// <param name="args">Arguments to pass all invoked methods</param>

public void InvokeAll(object[] args)
{
WeakDelegate current;
for(int i = 0; i < delegates.Count; i++)
{
current = delegates[i] as WeakDelegate;
if(current.IsAlive)
current.Invoke(args);
else
{
delegates.RemoveAt(i);
i--;
}
}
}

/// <summary>Add a WeakDelegate to the invokation list</summary>

/// <param name="d">WeakDelegate to add</param>

public void AddDelegate(WeakDelegate d)
{
delegates.Add(d);
}

/// <summary>Remove a WeakDelegate from the invokation list</summary>

/// <param name="d">WeakDelegate to remove</param>

public void RemoveDelegate(WeakDelegate d)
{
delegates.Remove(d);
}

private ArrayList delegates = new ArrayList(10);
}
}


Anyone who reads this is free to use the code. It would be nice if you let me know if you did though :-)

Thanks again for the last time everyone!

-SniperBoB-

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.

Sign in to reply to this topic.