Deterministic Disposable
Here is a challenge, get this to work:
///<summary> /// Executes the given handler when the instance is disposed /// of using the Dispose(instance); /// Note: Doesn't cause memory leak ///</summary> public void OnDisposable(object instance, Action<object> action); ///<summary> /// Executes the previously registered Action for this /// instance ///</summary> public void Dispose(object instance);
The key part here is to get this to work without causing a memory leak. Furthermore, assume that you need to handle this scenario as well without causing a leak:
object instance = new object(); OnDisposable(instance, delegate(object obj) { Console.WriteLine("Disposing of {0}", obj); });
Comments
Why the new object? Why isn't the second snippet:
OnDisposable(this, delegate(object)
{
Console.WriteLine("Disposing if {0}", obj);
});
This is before any managed or unmanaged memory or resources have been freed?
You are doing it to other objects, not to yourself
Define "leak": extra handle usage before GC.Collect, virtual memory increase, rooted object increase, etc?
Leak: memory consumption of the application goes up and never goes down.
Dictionary<object, Action<object>> actions = new Dictionary<object, Action<object>>();
Something to do with the fact that delegates are strong references?
That's still pretty vague. The Windows memory manager will show an application's usage of memory based on the number of pages that application has mapped to physical memory, despite the application having freed any and all memory in some of those pages. The memory manager won't recycle those pages until it needs to. You can force that to happen by minimizing and restoring the application.
Have a look at some other application's memory usage in task manager while they're not minimized then try minimizing then restoring them--you'll see the memory usage go way down (normally). this is because it has forced the memory manager to take those unused pages away from the application.
Eyal,
You are leaking references to the Action<object>
Peter,
Don't get technical on me. :-)
I am talking about memory that is not reclaimable by the GC. Is this tight enough definition?
By listening to an object using the standard event mechanism you are preventing it from ever being collected or disposed - i.e a lapsed listener.
In .NET 3.5 you can use the WeakEventManager and WeakEvent classes to get round this.
[)amien
Sounds like newly-rooted objects. How are you detecting these?
I'm guessing this is what you're after?
public class WeakDelegate
{
}
Ayende: good point. I need to think about it some more..
Geert: But where do you keep the instance of that WeakDelegate?
In a dictionary? You'll leak a DictionaryEntry in that case.
A list? You'll leak a list slot.
Why aren't you using IDisposable? Something like this:
public class Disposable<T> : IDisposable {
public Disposable(T instance, Action<T> action) { ... }
...
}
object instance = ...;
Action<object> action = ...;
using(new Disposable(instance, myAction)) {
...
}
Eyal, maybe he could modify WeakDelegate to raise an event in the case that target == null in his Invoke method and then handle that event to clear the list slot or dictionary entry.
Which means Invoke must be called in order to clean this up. The scenario Ayende gave is that nothing should leak even if Dispose isn't called.
I'm probably missing something here..
You're right. That would require that invoke be called in order to clean it up.
The only other way I can think of to make sure that the list entries are not leaked is to occasionally query the weak delegate to see if its taret has been GC'd. If so, then clean up the list entry.
However, that seems like a lot of overkill for this problem.
There must be a better way.
Without thinking this through, a solution could be to create a new object with a finalizer and wire it to the disposable object. When that object's finalizer gets called, I can assume that my disposable object has been finalized too or (the difficult part) will be finalized within the same GC cycle. So the first time my finalizer gets called, I have to resurrect myself (can I call GC.KeepAlive(this) in the finalizer?), or new up yet another object that gets GC'ed the next time around, and then do the call to the action delegate and remove my WeakReference stuff. Please take this with a grain of salt, but it could work. Keep in mind that objects with finalizers take two GC cycles to dissaper (finalize, then free the memory), so this might add up to several cycles before the memory actually gets freed.
If OnDisposable is called on an object, is Dispose guaranteed to be called on that object?
If Dispose is called on an object, is it guaranteed that OnDisposable has been called on that object before?
Is it guaranteed that for any object, OnDisposable and Dispose are called exactly zero or one times?
How open-ended is the scenario?
Jay,
No, Dispose() is not guaranteed to be called.
I guess that you could say that if you call Dispose you called OnDisposable, but that is not something that I want to be on.
Both may be call zero or more times.
OnDisposable overrwrite the action, Dispose run the action the first time and no op all else
As usual I probably do not understand the question, but nothing is leaking in this implementation:
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
namespace MemoryLeak
{
}
So the task is to implement a WeakDictionary<Key, Value> that has a weak reference to the key and frees the dictionary slot and value when the key has been garbage collected?
This is easy to do in Java (WeakHashMap is part of the standard library, but even if it wasn't, it could be easily implemented thanks to the ReferenceQueue class), but I don't know any good solution in .NET.
All .NET solutions involve some sort of periodically called dictionary cleanup, basically
foreach (WeakReference r in Keys) {
if (r.Target == null) Remove(r);
}
which of course isn't efficient when the dictionary is large.
Oh, now I get it.
Nick Guerrera implemented a generic dictionary, which allows both its keys and values to be garbage collected if there are no other references to them than from the dictionary itself back in 2006.
http://blogs.msdn.com/nicholg/archive/2006/06/04/617466.aspx
Alex,
What happens if you don't call Dispose() ?
Daniel,
Exactly :-)
As daniel said, you can create a thread that checks for collected refernces (which is what ASP.NET Cache does) but it seems like too much overhead.
Another solution is to create a CollectionNotifier clas:
class CollectionNotifier
{
~CollectionNotifier(Subscriber)
{
Subscriber.GCCalled();
}
}
Now you can do something like this:
new WeakRefernce(new CollectionNotifier(this));
And get notified when GC was called and references were removed and scan your own list for removed references.
What's your solution Ayende?
just in case anybody is still trying to make sense of my comment: there's none, I just really didn't think it through (confused two sides of a reference in regard to who's kept alive, now how embarrassing is that...)
I don't get it - Is there an answer to this question?? Or are you just posting questions you need help with? :)
Comment preview