tags:

views:

888

answers:

11

Hi guys,

I've created an "attached behaviour" in my WPF application which lets me handle the Enter keypress and move to the next control. I call it EnterKeyTraversal.IsEnabled, and you can see the code on my blog here.

My main concern now is that I may have a memory leak, since I'm handling the PreviewKeyDown event on UIElements and never explicitly "unhook" the event.

What's the best approach to prevent this leak (if indeed there is one)? Should I keep a list of the elements I'm managing, and unhook the PreviewKeyDown event in the Application.Exit event? Has anyone had success with attached behaviours in their own WPF applications and come up with an elegant memory-management solution?

A: 

Make sure event referencing elements are with in the object they are referencing, like text boxes in the form control. Or if that can't be prevented. Create a static event on a global helper class and then monitor the global helper class for events. If these two steps cannot be done try using a WeakReference, they are usually perfect for these situations, but they come with overhead.

Nick Berardi
+1  A: 

@Nick Yeah, the thing with attached behaviours is that by definition they're not in the same object as the elements whose events you're handling.

I think the answer lies within using WeakReference somehow, but I've not seen any simple code samples to explain it to me. :)

Matt Hamilton
A: 

I just read your blog post and I think you got a bit of misleading advice, Matt. If there is an actual memory leak here, then that is a bug in the .NET Framework, and not something you can necessarily fix in your code.

What I think you (and the poster on your blog) are actually talking about here is not actually a leak, but rather an ongoing consumption of memory. That's not the same thing. To be clear, leaked memory is memory that is reserved by a program, then abandoned (ie, a pointer is left dangling), and which subsequently cannot be freed. Since memory is managed in .NET, this is theoretically impossible. It is possible, however, for a program to reserve an ever-increasing amount of memory without allowing references to it to go out of scope (and become eligible for garbage collection); however that memory is not leaked. The GC will return it to the system once your program exits.

So. To answer your question, I don't think you actually have a problem here. You certainly don't have a memory leak, and from your code, I don't think you need to worry, as far as memory consumption goes either. As long as you make sure that you are not repeatedly assigning that event handler without ever de-assigning it (ie, that you either only ever set it once, or that you remove it exactly once for each time that you assign it), which you seem to be doing, your code should be fine.

It seems like that's the advice that the poster on your blog was trying to give you, but he used that alarming work "leak," which is a scary word, but which many programmers have forgotten the real meaning of in the managed world; it doesn't apply here.

DannySmurf
You are wrong. This is why applications like Ants Memory Profiler exist.
Anderson Imes
Again with the 14-month-late response... :)
DannySmurf
A: 

@Danny thanks for the explanation! You make a lot of sense - the memory might be getting consumed and not released while my app is running, but the garbage collector should give back the entire allocation to Windows when the program exits.

I'll live with it for now - this isn't code that's going to be hit often enough to run rampant while the app is running.

Matt Hamilton
+3  A: 

I do not agree DannySmurf

Some WPF layout objects can clog up your memory and make your application really slow when they are not garbadge collected. So I find the choice of words to be correct, you are leaking memory to objects you no longer use. You expect the items to be garbadge collected, but they aren't, because there is a reference somewhere (in this case in the from an event handler).

Now for a real answer :)

I advise you to read this WPF Performance article on MSDN

Not Removing Event Handlers on Objects may Keep Objects Alive

The delegate that an object passes to its event is effectively a reference to that object. Therefore, event handlers can keep objects alive longer than expected. When performing clean up of an object that has registered to listen to an object's event, it is essential to remove that delegate before releasing the object. Keeping unneeded objects alive increases the application's memory usage. This is especially true when the object is the root of a logical tree or a visual tree.

They advise you to look into the Weak Event pattern

Another solution would be to remove the event handlers when you are done with an object. But I know that with Attached Properties that point might not always be clear..

Hope this helps!

Arcturus
+1  A: 

Have you though of implementing the "Weak Event Pattern" instead of regular events?

  1. Weak Event Pattern in WPF
  2. Weak Event Patterns (MSDN)
Patrik
A: 

@Arcturus Ok, you've convinced me to look further into this. Sorry @DannySmurf but I'm gonna change the answer. :)

Matt Hamilton
A: 

@Arcturus:

... clog up your memory and make your application really slow when they are not garbage collected.

That's blindingly obvious, and I don't disagree. However:

...you are leaking memory to object that you no longer use... because there is a reference to them.

"memory is allocated to a program, and that program subsequently loses the ability to access it due to program logic flaws" (Wikipedia, "Memory leak")

If there is an active reference to an object, which your program can access, then by definition it is not leaking memory. A leak means that the object is no longer accessible (to you or to the OS/Framework), and will not be freed for the lifetime of the operating system's current session. This is not the case here.

(Sorry to be a semantic Nazi... maybe I'm a bit old school, but leak has a very specific meaning. People tend to use "memory leak" these days to mean anything that consumes 2KB of memory more than they want...)

But of course, if you do not release an event handler, the object it's attached to will not be freed until your process' memory is reclaimed by the garbage collector at shutdown. But this behaviour is entirely expected, contrary to what you seem to imply. If you expect an object to be reclaimed, then you need to remove anything that may keep the reference alive, including event handlers.

DannySmurf
Leak has a specific meaning in the context of a managed language versus an unmanaged one. The generally understood nomenclature refers to an object left in memory longer than its expected lifetime as a leak in managed code. You are correct in stating that in the unmanaged world when applications are not running in protected mode, memory not freed back to the OS cannot be reclaimed, but this has nothing to do with the OP's question and contributes nothing to the discussion.
Anderson Imes
Well, the discussion was over about 14 months ago. You're a bit late to criticize.
DannySmurf
Hah... fair enough. I thought it was 2 months ago, not 14 :)
Anderson Imes
+2  A: 

Yes I know that in the old days Memory Leaks are an entirily different subject. But with managed code, new meaning to the term Memory Leak might be more appropiate...

Microsoft even acknowledges it to be a memory leak:

Why Implement the WeakEvent Pattern?

Listening for events can lead to memory leaks. The typical technique for listening to an event is to use the language-specific syntax that attaches a handler to an event on a source. For instance, in C#, that syntax is: source.SomeEvent += new SomeEventHandler(MyEventHandler).

This technique creates a strong reference from the event source to the event listener. Ordinarily, attaching an event handler for a listener causes the listener to have an object lifetime that influenced by the object lifetime for the source (unless the event handler is explicitly removed). But in certain circumstances you might want the object lifetime of the listener to be controlled only by other factors, such as whether it currently belongs to the visual tree of the application, and not by the lifetime of the source. Whenever the source object lifetime extends beyond the object lifetime of the listener, the normal event pattern leads to a memory leak: the listener is kept alive longer than intended.

We use WPF for a client app with large ToolWindows that can be dragged dropped, all the nifty stuff, and all compatible with in a XBAP.. But we had the same problem with some ToolWindows that weren't garbadge collected.. This was due to the fact that it was still dependent on event listeners.. Now this might not be a problem when you close your window and shut down your app. But if you are creating very large ToolWindows with a lot of commands, and all these commands gets re-evaluated over and over again, and people must use your application all day long.. I can tell you.. it really clogs up your memory and response time of your app..

Also, I find it much easier to explain to my manager that we have a memory leak, than explaining to him that some objects are not garbadge collected due to some events that needs cleaning ;)

Arcturus
A: 

Well that (the manager bit) I can certainly understand, and sympathize with.

But whatever Microsoft calls it, I don't think a "new" definition is appropriate. It's complicated, because we don't live in a 100% managed world (even though Microsoft likes to pretend that we do, Microsoft itself does not live in such a world). When you say memory leak, you could mean that a program is consuming too much memory (that's a user's definition), or that a managed reference will not be freed until exit (as here), or that an unmanaged reference is not being properly cleaned up (that would be a real memory leak), or that unmanaged code called from managed code is leaking memory (another real leak).

In this case, it's obvious what "memory leak" means, even though we're being imprecise. But it gets awfully tedious talking to some people, who call every over-consumption, or failure-to-collect a memory leak; and it's frustrating when these people are programmers, who supposedly know better. It's kind of important for technical terms to have unambiguous meanings, I think. Debugging is so much easier when they do.

Anyway. Don't mean to turn this into an airy-fairy discussion about language. Just saying...

DannySmurf
A: 

True true,

You are right of course.. But there is a whole new generation of programmers being born into this world that will never touch unmanaged code, and I do believe language definitions will re-invent itself over and over again. Memory leaks in WPF are in this way different than say C/Cpp.

Or course to my managers I referred to it as a memory leaks.. to my fellow colleagues I referred to it as a performance issue!

Referring to the Matt's problem, it might be a performance issue that you might need to tackle. If you just use a few screens and you make those screen controls singletons, you might not see this problem at all ;).

Arcturus