Freigeben über


Controls are like diapers: you don't want a leaky one [Implementing the WeakEvent pattern on Silverlight with the WeakEventListener class]

**

This blog has moved to a new location and comments have been disabled.

All old posts, new posts, and comments can be found on The blog of dlaa.me.

See you there!

Comments

  • Anonymous
    March 09, 2009
    PingBack from http://blogs.msdn.com/delay/archive/2009/03/09/controls-are-like-diapers-you-don-t-want-a-leaky-one-implementing-the-weakevent-pattern-on-silverlight-with-the-weakeventlistener-class.aspx

  • Anonymous
    March 11, 2009
    In my last post, I explained how it was possible for "hidden" event handlers to introduce memory leaks

  • Anonymous
    March 15, 2009
    I thought that lines like: oldNotifyCollectionChanged.CollectionChanged -= OnCollectionChanged; prevents from leaking... it's confusing :P

  • Anonymous
    March 15, 2009
    LukeSw, Detaching from the CollectionChanged event when ItemsSource is changed is certainly necessary to avoid leaking. But it is not sufficient. What if the control is removed from the visual tree while its ItemsSource is still set? Then there's no indication that the control needs to unhook its event handler and leaks can result - unless the Weak Event pattern is employed.

  • Anonymous
    March 18, 2009
    We've just published the March 09 release of the Silverlight Toolkit and I bet there's something in there

  • Anonymous
    March 20, 2009
    @Dealy So basically speaking, Weak Event pattern is needed so the programmer don't have to remember to detach an even handler and/or ItemsSource property?

  • Anonymous
    March 20, 2009
    LukeSw, The Weak Event pattern doesn't exist so that programmers can be lazy - though it does make that a little easier. :) Rather, it exists because there are certain circumstances where a perfectly valid program can get itself into a situation where a control it created is leaked and there's virtually nothing the program itself can do to detect the problem or correct it. It's for these unavoidable scenarios that the Weak Event pattern exists and is used.

  • Anonymous
    March 21, 2009
    Delay, I analyzed your sample and it seems that the WeakEvent pattern is helpful only in those situations when before discarding an object, a programmer forgot (or doesn't know they must) to clear some properties (like ItemsSource) which attaches some handlers when being set. That's really tricky. It's a pity CLR doesn't do that automatically... It would be better if delegates used weak references (I know that now it's too late for their change/fix because of backward compatibility). quote: "What if the control is removed from the visual tree while its ItemsSource is still set? Then there's no indication that the control needs to unhook its event handler and leaks can result" I don't agree. Maybe I'll provide not very common example, but what if the control is only temporarily removed from visual tree? It wouldn't be useful if I had set ItemsSource whenever I want add back the control to the visual tree.

  • Anonymous
    March 21, 2009
    LukeSw, You might be surprised that I agree with what you're saying. :) For cases like the scenario I describe, I agree that it would be nice if delegates used weak references. But I bet there are some very good reasons why they don't - and besides, you're right that it's unlikely this will change now. Also, your example provides a good reason why - even if there were an event that was fired when a control left the visual tree - it would not be a good idea for a Control to null-out its ItemsSource. However, if such an event existed, it could be used by an application to do so - and then one might possibly argue that the Weak Event pattern was not strictly necessary. But that's still putting a lot of burden on the application developer and it's unreasonable to put that burden on ALL application developers. Therefore the Weak Event pattern exists and should be used by Controls when circumstances call for it.

  • Anonymous
    February 20, 2010
    Is it just me, or is the reference only detached if the object raises an event? What if that event never gets raised? As far as I can tell in my sample code, then the references will never get released.

  • Anonymous
    February 20, 2010
    SharpGIS, It's not just you; if the event is never raised again, the code above won't magically know to detach from the collection. If that's a problem in your scenario, the most direct solution would seem to be to introduce some kind of periodic polling to the code above. However, the overhead of that seems like it could be more significant than the liklihood of a leak due to the event never firing again. If you have other ideas here, I'd love to hear them! PS - In researching this reply, I came across the following article I hadn't previously seen: http://www.codeproject.com/KB/cs/WeakEvents.aspx. In it, Daniel Grunwald discusses a variety of different options in detail. However, it seems to me that none of the relevant options from Part 1 (excepting the WPF-specific one) address the issue you're asking about, either. PS - Stay tuned to my blog for an update on some some very recent work in this area  (though not directly related to this problem). :)

  • Anonymous
    February 20, 2010
    If you're interested, here's my sample code that demonstrates how badly this quickly can get: http://www.sharpgis.net/Samples/MemoryLeak.zip Just run the sample and watch the memory quickly grow every time it hits the simple timer_Tick method. I would love to hear your thoughts on how this leak could be remedied. I know I could get around it by implementing a Dispose() method, but it requires all developers to always remember to call this when they are done using the object.

  • Anonymous
    February 21, 2010
    SharpGIS, Our comments may have passed each other in "the tubes" - did you happen to see my "February 20, 2010 11:02 PM" comment above with a link to further details and a possible non-Dispose workaround? At any rate, I don't argue that this issue could be taken advantage of to create a significant leak in a contrived scenario - but do you think it's likely to cause significant problems in more realistic scenarios as well?

  • Anonymous
    January 19, 2011
    The comment has been removed

  • Anonymous
    January 20, 2011
    RScullard, Great info - thanks for following up! :)

  • Anonymous
    March 10, 2011
    Guys what the license for using WeakEventListener class in my code ?

  • Anonymous
    March 11, 2011
    The comment has been removed

  • Anonymous
    September 16, 2011
    Hi Delay - I have a question regarding implementation of the above. I have a SL navigation view with the following code (in the Loaded handler): ComboBoxA.ItemsSource = null; ComboBoxA.ItemsSource = App.StaticObservableCollectionA; App.StaticObservableCollectionA is an ObservableCollection<Entity> - stored in App.xaml as a public static property, like so: public static ObservableCollection<Business_Role> StaticObservableCollectionA { get; set; } question one - this looks like a scenario that will cause my view to stay in memory even though i've navigated away from it.  right? question two - if the answer to question one is yes, how do I use this pattern to alleviate this problem?  for better or worse, i have this all over my app... the memory creeps and creeps as i move to and from views in this app.  I'd like to put a lid on it.  Thanks for your help.   Kevin

  • Anonymous
    September 16, 2011
    Kevin, Yes, it seems like the reference to App.StaticObservableCollectionA will remain present even when the view isn't current. HOWEVER, the platform's ItemsControl class (upon which ComboBox is built) already does the right thing here to maintain weak references, so this shouldn't be the source of a leak for you (assuming this isn't a platform bug). That said, if you want to help be sure and cover all the bases, you should be able to set the ItemsSource to null in an Unloaded handler for your view (the counterpart to the Loaded handler you're already using). Alternatively, maybe do the attach/detach as part of the OnNavigatedTo/From process. You also might find it helpful to try to determine the source of the possible leak using the process I describe here: blogs.msdn.com/.../where-s-your-leak-at-using-windbg-sos-and-gcroot-to-diagnose-a-net-memory-leak.aspx Hope this helps!

  • Anonymous
    September 18, 2011
    Thanks for the response.  I'll set the source to null upon leaving the view for sanity's sake, and will be checking out windbg immediately. one last one - what about a simple, completely self-contained button click handler defined on my view for a button on my view?  does that need to be unregistered?  If the button is defined on the view it would seem that it should have the same lifetime as the view, and therefore shouldn't be a problem.  it will die when the view goes, regardless of the handler. is this correct?  or should ALL handlers be unregistered?   Thanks for your time. Kevin

  • Anonymous
    September 19, 2011
    Kevin, Your understanding is correct: if the thing subscribing to the event has the same lifetime as the thing that exposes the event, there shouldn't be a leak concern because both instances will become garbage at the same time and the fact that one references the other won't matter. The big concern is when there's a long-lived (i.e., for the life of the application) collection with one of these "backward references" to an individual page instance - especially if that new instances of that page are created regularly! :)

  • Anonymous
    September 19, 2011
    Thanks for your help David.  That clears up a lot.

  • Anonymous
    September 20, 2011
    Using WinDbg I was able to locate the source of my memory leak (good!).  Turns out it is caused by the use of a PanelDragDropTarget (bad!), as discussed here:  silverlight.codeplex.com/.../7356. Not sure how to resolve this one other than removing the panel, which will also remove functionality that our users are used to having. Thanks for the advice.  

  • Anonymous
    September 20, 2011
    Kevin, Sorry about the trouble! However, from the comments of the issue you link to, it looks like folks may have been able to fix the leak. If you have time to experiment, maybe that would resolve the leak without requiring you to abandon the panel?

  • Anonymous
    September 20, 2011
    Yes sir that's what I'm doing - trying to figure out how to use their patched DragDropTarget.  Looks like I'll have to grab the entire toolkit source unless there's something I'm missing (which is entirely likely).   I appreciate all the help David.  This would have been quite a bit more difficult without the back and forth. Kevin