52
Vote

ContextMenu causes memory leak

description

ContextMenu subscribes MouseMove event in ContextMenu.InitializeRootVisual, but it won't unsubscribe it, and it don't use WeakEvent. So it won't be GCed after it is unloaded. I verified the leak in Windbg, sample !gcroot result:
 
DOMAIN(063DF7F0):HANDLE(Pinned):6b612f8:Root: 07d64260(System.Object[])->
06d96964(SAPStudio.I264Player.SilverlightGui.MainPage_3)->
06d96a2c(MS.Internal.CoreTypeEventHelper)->
06d96a98(System.Collections.Generic.Dictionary2[[System.Int32, mscorlib],[MS.Internal.CoreTypeEventHelper+EventAndDelegate, System.Windows]])->
07907da0(System.Collections.Generic.Dictionary2+Entry[[System.Int32, mscorlib],[MS.Internal.CoreTypeEventHelper+EventAndDelegate, System.Windows]][])->
0765c110(MS.Internal.CoreTypeEventHelper+EventAndDelegate)->
0765c0d8(System.Windows.Input.MouseEventHandler)->
*** 0764ce5c(System.Windows.Controls.ContextMenu) ***->
0764de04(System.Windows.Controls.ItemCollection)->
07650ee0(System.Collections.Generic.Dictionary2[[System.Windows.UIElement, System.Windows],[System.Object, mscorlib]])->
07650f2c(System.Collections.Generic.Dictionary2+Entry[[System.Windows.UIElement, System.Windows],[System.Object, mscorlib]][])->
0764ec38(System.Windows.Controls.MenuItem)->
0764ecc0(MS.Internal.CoreTypeEventHelper)->
0764f4d8(System.Collections.Generic.Dictionary2[[System.Int32, mscorlib],[MS.Internal.CoreTypeEventHelper+EventAndDelegate, System.Windows]])->
0764f524(System.Collections.Generic.Dictionary2+Entry[[System.Int32, mscorlib],[MS.Internal.CoreTypeEventHelper+EventAndDelegate, System.Windows]][])->
0764f4c8(MS.Internal.CoreTypeEventHelper+EventAndDelegate)->
0764f490(System.Windows.RoutedEventHandler)->
0764f318(System.Windows.Data.BindingExpression)->
0764f144(SAPStudio.I264Player.SilverlightGui.Interactivity.ClipboardCommands.CopyCommand)

file attachments

comments

Mendoza32 wrote Oct 29, 2010 at 7:57 AM

I've uploaded simple fix for this issue. Look for patch ID 7215 in http://silverlight.codeplex.com/SourceControl/PatchList.aspx

adventful wrote Jan 22, 2011 at 5:11 AM

I also submitted a patch for this (8163) - didn't find this issue or Mendoza32's patch until I'd already made the fix, so I figured I might as well add mine as well.

rehansaeed wrote Feb 24, 2011 at 8:23 AM

This bug makes the ContextMenu unusable for me. If you use it on any page with a few images then eventually the application will crash out because there is no memory left. The two patches are useful but they require you to remove the ContextMenu when navigating away from the page. In my case this is very difficult as I am using the ContextMenu from within a ListBox's DataTemplate.

Thraka wrote Apr 20, 2011 at 4:14 PM

Why is the impact on this Low? This should be High!

dksimon wrote Apr 20, 2011 at 4:56 PM

If adventful's patch just removes the handler when it is unloaded, how will this work if the ContextMenu is in a TabControl? The Loaded/Unloaded events get called any time you switch tabs, this would seem to break it if I switch away from a tab that has a ContextMenu and back.

trenttobler wrote May 11, 2011 at 12:31 PM

We encountered this bug too, but took a slightly different tact -- we used a weak event listener pattern. Instead of:
            if (null != _rootVisual)
            {
                // Ideally, this would use AddHandler(MouseMoveEvent), but MouseMoveEvent doesn't exist
                _rootVisual.MouseMove += new MouseEventHandler(HandleRootVisualMouseMove);
            }
we used:
            if (null != _rootVisual)
            {
                var rootVisual = _rootVisual;

                // Use a weak event listener.
                var rootVisualMouseMoveListener = new WeakEventListener<ContextMenu, object, MouseEventArgs>( this );
                rootVisualMouseMoveListener.OnEventAction = ( instance, source, eventArgs ) => instance.HandleRootVisualMouseMove( source, eventArgs );
                rootVisualMouseMoveListener.OnDetachAction = ( weakEventListener ) => rootVisual.MouseMove -= weakEventListener.OnEvent;
                rootVisual.MouseMove += rootVisualMouseMoveListener.OnEvent;
            }
Probably the existing patches are better if they in fact track the context menu lifecycle and explicitly detach, rather than waiting for the event to trigger. When we evaluated this, we decided to just keep the weak event listener since we had already done testing and found it to fix the leak.

jcheung wrote Jun 17, 2011 at 1:30 AM

I also believes that the impact of this issue should be HIGH.

yifung wrote Jun 22, 2011 at 6:35 PM

You can try the attachment in workitem 6206
http://silverlight.codeplex.com/workitem/6206

anilchinnu21 wrote Sep 25, 2011 at 10:18 AM

Below is the ultimate solution that we found after looking into the codeplex community forum threads.

http://www.dotnetthread.com/articles/6-SolutiontoMemoryleakissueswithSilverlightcontroltoolkit.aspx

anilchinnu21 wrote Sep 25, 2011 at 10:19 AM

Below is the ultimate solution that we found after looking into the codeplex community forum threads.

http://www.dotnetthread.com/articles/6-SolutiontoMemoryleakissueswithSilverlightcontroltoolkit.aspx

nico008 wrote Feb 13, 2012 at 7:37 AM

Anyone has a better solution than patch ID 7215?
I don't like much having to call something from OnNavigatedFrom...

Note : this issue should have medium or high impact since this cause all my pages not GCed -> my app reach GBs after some hours.

nico008 wrote Feb 13, 2012 at 1:54 PM

Sorry, I missed anilchinnu21's post.
His link solved two big memory leak I had.

Great work!

I hope this will be in next Toolkit release

Xiaoyue2838 wrote Mar 8, 2012 at 2:14 PM

The source code in Silverlight SDK uses a defferent PublicKeyToken, after replacing reference to the modified System.Windows.Controls.Input.Toolkit.dll the build is failed. The error message is The type 'System.Windows.Controls.ExpandDirection' is defined in an assembly that is not referenced. You must add a reference to assembly 'System.Windows.Controls.Toolkit, Version=5.0.5.0, Culture=neutral, PublicKeyToken=2c5c654d367bf4a7'. How to resolve this problem?

Xiaoyue2838 wrote Mar 9, 2012 at 1:47 PM

The error message is resolved by replacing more references to the new build dlls which use same new public key token.

kingpin2k2 wrote Jun 18, 2012 at 6:48 PM

At the very least allow it to be manually disposed, implement IDisposable or something.

/// <summary>
    /// Remove events and allow the Element to be GC'd
    /// </summary>
    public void Dispose()
    {
        //This will remove the right click event
        Owner = null;

        //This will remove the mouse move event
        if (null != _rootVisual)
        {
            _rootVisual.MouseMove -= HandleRootVisualMouseMove;
        }
    }

jandorfer wrote Jun 29, 2012 at 10:24 PM

The WeakEventListener solution linked to by anilchinnu21 appears to be a dead link now (though still available in google's cache). It's also a little out of date and requires minor modification to work with the latest source.

I made an updated patch derived from that solution and uploaded it here. Look for patch #12505 (download link: http://www.codeplex.com/Download?ProjectName=silverlight&DownloadId=418566).

beffes wrote Jul 13, 2012 at 4:38 PM

I compiled all the toolkit but I'm having trouble with SYSTEM.WINDOWS.CONTROL, because when I add a reference to it it stays with an alert icon above it.

Any thoughts on how to get it reference withou error?

beffes wrote Jul 13, 2012 at 4:40 PM

here is the printscreen:

Sign in to add a comment or to set email notifications