4

I have a big solution with graphics, popups and animations. I have found out that I have a massive memory leak during page navigations.

Tries

I therefore tried with the first solution:

protected override void OnNavigatedTo(NavigationEventArgs e)
    {
        if (App.RootFrame.CanGoBack)
            App.RootFrame.RemoveBackEntry();
        GC.Collect();
        base.OnNavigatedTo(e);
    }

This from several sources on MSDN and Stackoverflow, should remove the memory stored for the page. This was not the case and I am unsure if the MVVM structure of the code somehow keeps information stored. I then tried to implement deconstructors and force values to null when the event was fired like:

~SecondScreen()
    {
        In_Game_Crest = null;
        currentViewModel = null;
    }

This I did for all pages, popups and usercontrols. I then again went through the code using debug, and non of the pages deconstructor was ever fired. This has lead me on to try and use IDisposable and fiddeling around with the viewmodelLocator provided by MVVMLight, without any success.

Investigation

I have read the following addressing the issue: StackOverFlow: Finalizer and Dispose

Finalize/Dispose pattern in C#

MSDN: Implementing a Dispose Method

MSDN: Implementing Finalize and Dispose to Clean Up Unmanaged Resources

Questions

But it has confused me more than it helped me. How should I implement the dispose and finalize methods for a page for my windows phone?

Since I'm using the MVVM structure should these methods be implemented in a ViewModel or behind the given page or both?

Examples for windows phones would be much appreciated.


Initial try with Dispose

I have read some more about the subject and found that the finalize maybe shouldn't be written? But I am still unsure. But based on this and the first MSDN link above, I tried the following:

private bool disposed = false;

    public void Dispose()
    {
        Dispose(true);
        // Take yourself off the Finalization queue 
        // to prevent finalization code for this object
        // from executing a second time.
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
  // Check to see if Dispose has already been called.
  if(!this.disposed)
  {
     // If disposing equals true, dispose all managed 
     // and unmanaged resources.
     if(disposing)
     {
        // Dispose managed resources.
         currentView = null;
         popup = null;
         Image1 = null;
         Image2 = null;
     }
          // Release unmanaged resources. If disposing is false, 
          // only the following code is executed.
          this.Content = null;
          // Note that this is not thread safe.
          // Another thread could start disposing the object
          // after the managed resources are disposed,
          // but before the disposed flag is set to true.
          // If thread safety is necessary, it must be
          // implemented by the client.

  }
  disposed = true;         
    }

    // Use C# destructor syntax for finalization code.
    // This destructor will run only if the Dispose method 
    // does not get called.
    // It gives your base class the opportunity to finalize.
    // Do not provide destructors in types derived from this class.

    ~FirstPage()
    {
        Dispose(false);

    }
    protected override void OnNavigatedFrom(NavigationEventArgs e)
    {
        this.Dispose();
        base.OnNavigatedFrom(e);
    }

But, but but this just made my memory increase by 23MB when I got to the second screen. This leads me to the question again, how and what should I try to implement, and WHY could the memory increase?

this = null, base.Dispose()

I have seen different implementations, either using this = null in the dispose function or using base.Dispose(). I figure that the latter can only be used if the class is IDisposable? Is this the way to go? If so how do I go about it?

Using Microsoft Profiler

So I used the profiler to verify that the FirstPage is not removed enter image description here From the above Figure it can be seen that the firstpage exists. In the comments I was told to look for the instances and reference to the elements. I therefore chose instances of firstpage and got: enter image description here Here it is confirmed that FirstPage is never destroyed. But I am stuck here, how should I interpret the data? Hoping for some help.

Community
  • 1
  • 1
JTIM
  • 2,774
  • 1
  • 34
  • 74
  • 5
    For god's sake: Please don't use `GC.Collect` in production code unless there is a compelling reason to do so. – Sriram Sakthivel Jan 02 '15 at 15:04
  • 1
    For memory problems, use a memory profiler and find why objects live which you believe shouldn't be alive. – Sriram Sakthivel Jan 02 '15 at 15:06
  • @SriramSakthivel I know it is not the preferred solution is in c# this should be done automatically. BUT as I had issues, I TRIED the solutions. Please explain what a memory profiler is and refer me to one. I only use a live updated memory such that I can test the app live and see the memory usage – JTIM Jan 02 '15 at 15:15
  • [Refer this](http://blogs.msdn.com/b/dotnet/archive/2013/04/04/net-memory-allocation-profiling-with-visual-studio-2012.aspx) for profiling help. Visual studio comes with profiler, do some research about it. It is hard to help the memory problems. You really need to use profiler. – Sriram Sakthivel Jan 02 '15 at 15:20
  • @SriramSakthivel I have tested the application using the build in memory profiler. This has however not helped as I have found the issue of the memory leak, i.e. pages and popups not being deconstructed. Please Refer to the question on how to accomplish this. Our though has been to try and use IDisposable – JTIM Jan 02 '15 at 16:12
  • What do you meant by deconstructed? If you found out your objects are alive, profiler should tell you why they are alive, Which object holds the reference to them etc. That should indeed help. – Sriram Sakthivel Jan 02 '15 at 18:10
  • I mean the function that should be fired during a GC of a class/page. I have used the profiler. But I have not been able to deduct anything other than the page exists. As you say some reference must exist. I have gone through the code removed statics, and made sure that no reference should be there. I also did the same with the viewmodel. I therefore wanted to know if there was a special issue with MVVM or if I could fire the deconstructor somehow, that was why I was looking at IDisposable. Since it is a known issue with pages in Windows phone that they can result in memory leak. – JTIM Jan 02 '15 at 23:39
  • The fact that you are calling `RemoveBackEntry` as a mechanism to save memory is also an indication that something might be wrong with the way the app is structured. That API is primarily designed for things like "wizard" or "shopping cart" scenarios where the user goes through a set of pages and then you want to erase them for *usability* reasons, not for *resource usage* reasons. Can you explain the structure of your app, what kinds of resources are being held on to, etc? And as Sriram says, the VS profiler should tell you who is holding onto your pages (or where the real leak is). – Peter Torr - MSFT Jan 05 '15 at 02:03
  • Regarding the removebackentry, it was a solution someone suggested on msdn for the problem of pages not being cleared completely from memory this however did not solve the issue for me. The reason I asked about the IDisposable is that I found some references to the fact that there is an issue with memory leak in some pages on the phone. – JTIM Jan 05 '15 at 08:52
  • Regarding the structure, I have an intro page with logon. After user is verifyed a popup comes up and covers everything until the main page is loaded. From here you can navigate to many different pages, shops, design and games. All of which individually had low memory consumption around 50mb. Now that I navigate between it spikes and crashes. Maybe I should only rely on the VS profiler, but through my tries and the help I could find for it, it was not possible for me to determine the leak. Through breakpoints however I found that the deconstructor was never fired, which is why I look at IDispo – JTIM Jan 05 '15 at 08:56
  • 1
    @JTIM You have to remember, using `Dispose` or writing a finalizer will not help you if there's some root object keeping a strong reference to your object. The way to go isn't to implement any of these. What you should do is download a memory profiler [start here](http://memprofiler.com/) and try to find the roots that are keeping your objects alive. – Yuval Itzchakov Jan 10 '15 at 14:39
  • @YuvalItzchakov Okay, thank you I will try to look their. My idea with dispose was just that I could call it force the memory to be removed? I will try the link you referred. – JTIM Jan 10 '15 at 14:59
  • @JTIM `Dispose` does not cause memory to be released. It is merely a contract to calling code guaranteeing that you will release any unmanaged resources (such as native file handles) which are not under the CLRs restrictions – Yuval Itzchakov Jan 10 '15 at 16:31
  • @YuvalItzchakov but correct me if I'm wrong that would remove some of the memory usage right? Which is why I was interested in it. Would you have any comments to the latest update about the profiler in my question? – JTIM Jan 10 '15 at 16:42
  • @JTIM - Are you freeing any unmanaged memory? – Yuval Itzchakov Jan 10 '15 at 18:39
  • @JTIM You are holding a reference to `FirstApp` inside your viewmodel, right? Do you actually need that reference? – Yuval Itzchakov Jan 10 '15 at 19:06
  • @YuvalItzchakov I have removed that reference, and only hold a reference from the view to the viewmodel, this I have tried to set to `null` when navigating away, but with no help. – JTIM Jan 12 '15 at 13:59

2 Answers2

1

It is not actually necessary to dispose when a user navigates away from a page, the performance
implications of creating objects all over again is more than the memory load of having a page in the memory during when the application is alive.
You should take a decision between removing objects from the memory vis.a.vis recreating the same set of objects again.
Having said this you should be careful with the navigation model. Memory problems might occur if you are creating objects every time the user is navigating to a page but not actually disposing when the user navigates away.
For this purpose I recommend fully understanding the PageBase and NavigationHelper or NavigationService class in your application. You have mentioned that FirstPage is not removed from the memory during the life time of the app which according to me is ideal.
Place debug points in the potential places in your code where heavy objects might get created; navigate a couple of times to different Pages, then come back.
Check the behavior then you might get a clear picture for yourself.
For all objects check that you manually invoke Dispose.
Dispose is a completely different concept than GarbageCollector, Dispose is just an contract that developers should adhere to by invoking it for releasing resources they perceive are no longer required to be maintained in the memory since garbage collection by the platform takes place at a indeterminate time.
In the sample you have posted I see that you are setting objects to null without actually disposing. Setting to null just changes the memory location that variable is pointing to. It does not destroy the object immediately. an ideal dispose should be like below.

//Call on OnClosing or OnExit or similar context
protected override void Dispose(bool isDisposing)
{

        if(isDisposing && !_isDisposed){
         if(disposeableImage != null){
           disposeableImage.Dispose();
           disposeableImage = null;
         }
        }
}
Vignesh.N
  • 2,618
  • 2
  • 25
  • 33
  • Thank you for your reply. The firstpage is merely a login page with some animation. Therefore all the graphics and page should be removed after navigating away. I apologize for not making this clear. Regarding the dispose, many of the elements has not in there base a dispose function. My question is therefore also how to create the dispose in the below classes such that these will be removed completely. As you say null is not enough therefore I am interested in what to do in this case. Thank you for your in depth answer. – JTIM Jan 11 '15 at 16:03
  • if classes don't have Dispose its ok just to set references to `null` making sure that you don't hold any references in memory which might prevent object from being GC'd. In you case you can check that a page you no longer need is not referenced anywhere in your code, for example the navigation stack. – Vignesh.N Jan 11 '15 at 16:19
  • Okay thank you for that info, for some reason when I did set it to null the memory increased? I have tried and gone through the code and I should not have any reference, when I navigate away, since I remove the backstack upon entering newpages, this is because I want the application to not rely on the backkey, as this will not continue to be available. But for some reason the page is kept in memory long after I have navigated away in despite of me having set the elements to `null`. Frustrating. I will try and understand the profiling in more depth. – JTIM Jan 12 '15 at 13:57
  • I will award you the 100 points so they do not go to waste, but it is not an answer. That solved the issue, unfortunately for me :/ – JTIM Jan 12 '15 at 13:58
0

So what I did to solve the page memory leak was using the answer at this question:

Remove Pages windows phone

There still exists some leak, which helped when removing the storyboards and eventhandlers, and adding them and removing them. But some memory is still there but no leak is occurring.

Community
  • 1
  • 1
JTIM
  • 2,774
  • 1
  • 34
  • 74