Reputation: 13
I've writte a simple test tool using the .net FileSystemWatcher class. The problem is I got a memory leak because instances of MemoryLeakTest are referenced by the changed handler in the FileSystemWatcher. What would be the correct way of cleaning up this reference so that the garbage collector could afterwards collect the MemoryLeakTest instance and the FileSystemWatcher instance?
To see the leaked instances on the heap follow this instructions: http://blogs.msdn.com/b/calvin_hsia/archive/2008/04/11/8381838.aspx
Thanks in advance for your advice
using System;
using System.IO;
namespace MemoryLeakTest {
class Leaking {
private FileSystemEventHandler changedHandler;
private FileSystemWatcher fsw;
public Leaking() {
changedHandler = new FileSystemEventHandler(fsw_Changed);
fsw = new FileSystemWatcher("c:\\", "*.*");
fsw.Changed += changedHandler;
fsw.EnableRaisingEvents = true;
}
~Leaking() {
fsw.Changed -= changedHandler;
fsw.Dispose();
}
void fsw_Changed(object sender, FileSystemEventArgs e) {
Console.WriteLine("Changed");
}
}
class Program {
static void Main(string[] args) {
for (int i = 0; i < 100; ++i) {
var x = new Leaking();
}
GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
Console.ReadLine();
}
}
}
Upvotes: 1
Views: 2431
Reputation: 21
You may also be running into a memory leak in the FileWatcher itself. See the following: http://connect.microsoft.com/VisualStudio/feedback/details/565720/memory-leak-in-system-io-filesystemwatcher
Upvotes: 2
Reputation: 134045
Yes, you should implement IDisposable
in every class that has a member that implements IDisposable
. See http://msdn.microsoft.com/en-us/library/b1yfkh5e(v=VS.100).aspx.
However, implementing IDisposable
just makes sure that resources are cleaned up in a timely manner. Typically, failure to implement IDisposable
won't result in a long-term memory leak because the garbage collector will eventually get around to running the finalizers, and those will release the operating system resources. But not disposing can cause short-term memory leaks and resource exhaustion. It can cause a long-term problem, but that's relatively uncommon when working with BCL objects.
For example, in your test program above, the Leaking
objects that you create are "rootless" once you exit the loop, and as you saw the garbage collector does collect them. If you were to let your program run for a while, doing other things, those objects would be collected during the next garbage collection. It's a short-term memory and resource leak, but not a long-term problem.
Now, you could run into a memory leak if you have something like:
FileSystemWatcher watcher = new FileSystemWatcher(...);
void MakeABigLeak()
{
for (int i = 0; i < 10; ++i)
{
var MyObject = new SomeObject();
watcher.Changed += MyObject.ChangeHandler;
}
}
In this case, the watcher
holds a reference to each of the created objects. And since watcher
is rooted, those objects will remain active. They won't be garbage collected.
The only way to fix this is to ensure that the object removes itself from the event notification. You can do that in the object's Dispose
method, although there are other ways, too. However, you have to be careful not to do that in the finalizer. Remember that finalizers run in no set order. It's possible that the watcher
was finalized before the object that references it (since neither is rooted, finalization order doesn't matter). That's why the Dispose(bool)
method exists: to prevent you from accessing other objects during finalization.
It's your choice. You can write a Dispose
method that removes the object from the event notification, or you can be sure to write code that does the removal when the object is no longer interested in the event. The drawback to doing this in Dispose
is that the object doing the subscribing has to maintain a reference to the object that's raising the events.
Upvotes: 1