2

I am tracking down a memory leak in some applications. Many forms share the same Spellchecker object that therefore out lives any individual form. I am aware that this can be a cause of memory leaks if the handler events are not removed properly.

AddHandler and RemoveHandler make sense to me as it's clear if AddHandler is called then there should be a corresponding RemoveHandler. However does the Handles keyword automatically remove the handlers for you?

Private Sub spellingContextMenu_Popup(ByVal sender As Object, ByVal e As System.EventArgs) Handles spellingContextMenu.Popup

In the above if the spellingContextMenu lives for a long time but the form dies then should the handler be removed manually?

Microsoft's own page offers no guidance on this http://msdn.microsoft.com/en-us/library/6k46st1y.aspx#feedback

Alan Macdonald
  • 1,872
  • 20
  • 36
  • Btw, related: http://stackoverflow.com/questions/2207541/eventhandler-and-memory-leaks and http://stackoverflow.com/questions/2208775/withevents-handles-better-that-remove-addhandler – Tim Schmelter Sep 03 '13 at 11:17

1 Answers1

5

The Handles keyword is based on the assumption that normal event subscription practices can work, with the garbage collector taking care of collecting the event source, event subscriber and delegate object. Which works fine when the event source doesn't out-live the subscriber. A good example is controls inside a Form, when the form is closed then all controls inside of it die as well. So the next garbage collection gets rid of all of them, including any delegate objects created by Handles. No need to explicitly unsubscribe.

This doesn't sound like your case if your ContextMenu out-lives the form that uses it. And you do have a GC problem, your ContextMenu keeps a reference on the form through its event handler(s) so it won't be collected until the menu is collected. So using Handles is not the proper solution, you really do need to explicitly call AddHandler and RemoveHandler.

Do keep your eyes on the ball, is there really any point at all in not just creating a new ContextMenu for each form? That trivially solves your problem. An alternative is to not use events but have the form implement an interface. You register the form object with the spell checker class, it can listen for the Disposed event to know that it should drop the reference and stop making callbacks through the interface.

And beware of a bug in VB.NET, if you build the Debug version of an assembly then it will leak a WeakReference for every event that's declared WithEvents if you run it without a debugger attached. It is one that's created to implement Edit + Continue support, you must deploy the Release build of your assemblies to avoid this leak.

Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536
  • Thanks that's the information I am looking for. I didn't design/write it so I will consider whether to just create new objects etc. per form but I am lead to believe moving to one global one was to solve a memory issue int he first place rather than each form have it's own but that may have been a bad call. I will have to investigate further. – Alan Macdonald Sep 03 '13 at 14:26