none
Memory leak from MSDN sample

    Question

  • This sample on suspending an app seems to cause a leak - http://msdn.microsoft.com/en-us/library/windows/apps/xaml/hh465115.aspx

    partial class MainPage
    {
       public MainPage()
       {
          InitializeComponent();
          Application.Current.Suspending += new SuspendingEventHandler(App_Suspending);
       }
    }

    This stores a reference to MainPage in the app so each time you navigate back to it you leak an instance, plus the SuspendEvent will call the handler for each instance.  True?

    Wednesday, October 24, 2012 8:02 PM

Answers

  • that is absolutely correct, that's why you would generally subscribe to Application level events (like sized changed, datarequested etc.) in the OnNavigatedTo function and unsubscribe from the OnNavigatingFrom if you need to handle this event on the page level.

    Or you can implement a singleton class which does the event subscriptions for you and makes sure there is always one event handler attached to aforementioned events. (i.e. keep the event handler in a delegate while attaching and clear up the event before attaching another handler - possibly from a different page instance)


    Can Bilgin
    Blog CompuSight


    • Edited by Can BilginMVP Thursday, October 25, 2012 1:05 PM
    • Marked as answer by Payne Cash Friday, October 26, 2012 4:17 PM
    Thursday, October 25, 2012 1:05 PM

All replies

  • I think that can be a mistake and the MainPage should be App. When you create an project from VS2012 it brings this subscribed and is contained in App.xaml.cs.

    But i don´t want to say any wrong information.


    Sara Silva
    My Windows 8 Store Apps Samples
    Follow me in Twitter @saramgsilva

    My Windows 8 Store Apps: Female Pill | Galinho (Tic tac Toe) | 24
    (If my reply answers your question, please propose it as an answer because it will help other users)

    Wednesday, October 24, 2012 8:29 PM
  • "so each time you navigate back to it you leak an instance": in fact, this should be phrased "each time you instantiate a MainPage", since the Suspend handler is referenced anew in 'MainPage.Constructor' and not in 'MainPage.OnNavigateTo' method.

    Hence, the above construction should be safe.

    Thursday, October 25, 2012 5:22 AM
  • @Sara - thanks for your comment however in this case a specific page needs to know about the suspend event.

    @ForInfo - The page gets constructed each time you navigate to it and never gets finalized since the app now has a reference to the page via the suspend handler.

    Thursday, October 25, 2012 12:46 PM
  • There is a class in Grid project template that Help you and you do not implement it. 

    Sara Silva
    My Windows 8 Store Apps Samples
    Follow me in Twitter @saramgsilva

    My Windows 8 Store Apps: Female Pill | Galinho (Tic tac Toe) | 24
    (If my reply answers your question, please propose it as an answer because it will help other users)

    Thursday, October 25, 2012 12:55 PM
  • that is absolutely correct, that's why you would generally subscribe to Application level events (like sized changed, datarequested etc.) in the OnNavigatedTo function and unsubscribe from the OnNavigatingFrom if you need to handle this event on the page level.

    Or you can implement a singleton class which does the event subscriptions for you and makes sure there is always one event handler attached to aforementioned events. (i.e. keep the event handler in a delegate while attaching and clear up the event before attaching another handler - possibly from a different page instance)


    Can Bilgin
    Blog CompuSight


    • Edited by Can BilginMVP Thursday, October 25, 2012 1:05 PM
    • Marked as answer by Payne Cash Friday, October 26, 2012 4:17 PM
    Thursday, October 25, 2012 1:05 PM
  • @ForInfo: not when you navigate "back" but every time you navigate to the mainpage the Constructor is called, is this correct? And you keep on attaching new handlers to the Suspending event.

    You can have a simple experiment by attaching a event handler to SizeChanged or data requested (some application level event) which will pop a message dialog, and navigate to MainPage twice. The second time you would get a com exception that generally happens when there is an un-dismissed message dialog and you try to open the second one.

    So I would be reluctant to say that the construction is save. Or am I missing something?


    Can Bilgin
    Blog CompuSight

    Thursday, October 25, 2012 1:14 PM
  • @Payne Cash: my answer was indeed quite incomplete and my thanks for pointing that out.
    - with NavigationCacheMode=Disabled, the navigation in Grid App is 'type' oriented, not 'instance' oriented; when using a fresh GridApp without further ado there are two instances of GroupedItems in existence after a 'outward-inward' navigation sequence.
    - with NavigationCacheMode=Required, the same scenario uses a single GroupedItems instance
    - with NavigationCacheMode=Enable, one of these scenarios might occur
    - hence,  the construction is safe when NavigationCacheMode=Required and not safe when Disabled or Enabled.

    @Can:
    - "every time you navigate to the mainpage the Constructor is called, is this correct?": [see prior].
    - using one of your two suggested solutions would definitely be a wiser design decision [on a side node, this is in addition to 'OnNavigatedFrom' cleanup requirements as in 'Navigation : To cache or not to cache'].

    Friday, October 26, 2012 8:32 AM
  • Thanks for the clarification @ForInfo.  Back to the point of the original post - the sample is wrong or at least makes some unspecified assumptions so I will add community content suggesting @Can's solution of OnNavigated To/From.  I followed the sample and ended getting like 10 breaks on the suspend handler (after a lot of navigating to/from the page).
    Friday, October 26, 2012 4:17 PM
  • If I might add - I've been looking at sources of memory leaks in my Win 8 Split App as well.  Here's my experience:

    I tried using the OnNavigated To/From solution as described above, but it misses one important scenario: application suspend.  On a suspend, you get the NavigatedFrom event; on resume you DO NOT get the Navigated To event.  If you use these events to do event handler setup/cleanup, you also have to then make sure that your handlers are also properly setup in the resume event.

    I didn't want to register for yet another event that could cause memory leaks, so I went with the other approach suggested above - the singleton object that detaches old handlers prior to it attaching new ones.  Just my 2 cents.


    • Edited by chue12 Tuesday, November 06, 2012 5:52 PM
    Tuesday, November 06, 2012 5:41 PM