Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

UWP: Webview: Memory Leak #4720

Closed
csk1nner opened this issue Dec 12, 2018 · 14 comments · Fixed by #12509
Closed

UWP: Webview: Memory Leak #4720

csk1nner opened this issue Dec 12, 2018 · 14 comments · Fixed by #12509
Labels
a/performance a/webview blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. e/5 🕔 5 in-progress This issue has an associated pull request that may resolve it! p/UWP partner/cat 😻 t/bug 🐛 up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!
Milestone

Comments

@csk1nner
Copy link

Description

Using Xamarin.Forms (3.4.0.1008975), for UWP (windows 10) the webview control will take more and more memory space until it crashes the application

Steps to Reproduce

  1. Create a simple Xamarin.Forms application where page A navigates to page B that encapsulates a webview
  2. Run the application and note the memory in Task Manager that the application is consuming
  3. Navigate to the page that contains the webview. Notice that Task Manager is increasing the memory footprint of the application.
  4. Navigate back to the root (page A). Notice that the memory footprint in task manager is not fully recycled.
  5. Repeat steps 3 & 4 until application crashes

Expected Behavior

The webview control should closely mimic the resource allocation of Edge browser

Actual Behavior

After three or four navigation cycles (depends on webpage) the application crashes.

Basic Information

  • Version with issue: 3.4.0.1008975
  • Last known good version:
  • IDE: Visual Studio 2017
  • Platform Target Frameworks:
    • iOS:
    • Android:
    • UWP: 17134
  • Android Support Library Version:
  • Nuget Packages:
  • Affected Devices:
    UWP (Windows 10)

Screenshots

http://dynamiconnections.com/downloads/screenshot.png

Reproduction Link

http://dynamiconnections.com/downloads/navigationsample.zip

@csk1nner
Copy link
Author

Hello there!

Hope you are having a good day. Any news on this? I'm curious to know if there is a work around for this.

Cheers,

Caleb

@PureWeen
Copy link
Contributor

PureWeen commented Jan 6, 2019

Confirmed

image

@PureWeen PureWeen added the e/5 🕔 5 label Jan 6, 2019
@PureWeen PureWeen moved this from New to Ready For Work in Triage Jan 6, 2019
@csk1nner
Copy link
Author

csk1nner commented Jan 6, 2019

Thanks mate!

@samhouts samhouts added this to To do in UWP Ready For Work Jan 7, 2019
@samhouts samhouts removed this from Ready For Work in Triage Jan 7, 2019
@cheles
Copy link

cheles commented Mar 15, 2019

hi @samhouts. any updates on this? it looks like this was reported on UWP standalone apps too, not only XF and it looks like webview controller is leaking some memory in Back/Forward stack.

@CannoliSquid
Copy link

Any updates on this issue?

@cheles
Copy link

cheles commented Sep 20, 2019

Just an update that might impact XF too.
We released UWP 6.2.9 NuGet update with huge improvements in memory handling and GC calls while targeting >=RS3. The complete release notes are here: https://github.com/microsoft/dotnet/blob/master/releases/UWP/net-native2.2/README.md
@csk1nner can you try to update everything to latest and see if you still repro memory leak while navigating between pages?

@csk1nner
Copy link
Author

The memory leak is still in place. Same issue. Massive increase in memory on UWP. Android and IOS work fine.

@cheles
Copy link

cheles commented Oct 25, 2019

my tests showed memory handling improvements. Are you sure you've targeted >=RS3? or else, can you share a repro?

@csk1nner
Copy link
Author

Hello there,

Spun up a fresh tabbed xamarin.forms app and experienced the same problem. Added a webview control to the ItemDetailPage and was able to get the memory leak to occur. Please see attached code. Using the latest updates with pre-releases. Problem is occurring on UWP.

Please let me know what to change up to get this working.

Cheers,

Caleb

MemoryTest.zip

@csk1nner
Copy link
Author

One more thing. I think this issue has something to do with video players. When I browse to a somple website like: http://www.xamarin.com/ - There are no problems. Complex (normal news sites) crash this.

Cheers,

Caleb

@samhouts samhouts moved this from To do to To do (high priority) in UWP Ready For Work Feb 18, 2020
@samhouts samhouts added inactive Issue is older than 6 months and needs to be retested help wanted We welcome community contributions to any issue, but these might be a good place to start! up-for-grabs We welcome community contributions to any issue, but these might be a good place to start! and removed inactive Issue is older than 6 months and needs to be retested help wanted We welcome community contributions to any issue, but these might be a good place to start! labels Jul 17, 2020
@samhouts samhouts added this to the 5.0.0 milestone Aug 5, 2020
@samhouts samhouts added this to To do in vNext+1 (5.0.0) Aug 11, 2020
@csk1nner
Copy link
Author

csk1nner commented Sep 8, 2020

Gang! I've got an app trending in the Microsoft store (Unfiltered.me) that really needs this fix. Is there anything I can do to move the ball forward? Do you have some pre-release builds that I can test? I I code the fix?

@t-johnson
Copy link
Contributor

t-johnson commented Sep 29, 2020

I was curious about this issue and have had experience finding these kind of memory issues in the past.
Looked into the various WebView components for each platform for event handlers that may not have been removed.

I see a difference in the UWP WebViewRenderer (Xamarin.Forms\Xamarin.Forms.Platform.UAP\WebViewRenderer.cs) when compared to the other platforms. This ones Dispose() does not remove the event handlers that get added to the Element in OnElementChanged, whereas all the other platforms seem to do this.

Its been my experience that unremoved delegates can cause the GC to hold onto the objects, even when one might expect that they would go away themselves.

I'd be willing to try this out however i've never contributed to XF before so not sure how to go about building and testing a locally build dll.

Edit- Manual testing in the control gallery (using youtube.com as the website) suggests this change is good... before the change i could only navigate to the webview page 3 or 4 times before running out of memory. after the change, got no problems at all. if i can figure out how to encode this in a test case, i'll do a PR in the next day or so.

t-johnson pushed a commit to t-johnson/Xamarin.Forms that referenced this issue Oct 2, 2020
Add option for custom renderers to control the execution mode of the WebView control
This is 'opt-in' as by default this commit will not change behavior of existing applciations.  to opt-in, people would need to set the ExecutionMode property in the constructor of their custom WebViewRenderer, like:

  public class MyWebViewRenderer : WebViewRenderer
  {
    public MyWebViewRenderer()
    {
      ExecutionMode = Windows.UI.Xaml.Controls.WebViewExecutionMode.SeparateProcess;
    }
  }

When set as 'SeparateProcess', the memory allocated by the WebView itself is all handled in a sub-process, which ensures the main process never crashes from running out of memory here from doing things like opening and closing youtube every 5 seconds.  This behavior will likely crash when the WebView is in-process due to the huge amounts of memory that is required by that website.
t-johnson pushed a commit to t-johnson/Xamarin.Forms that referenced this issue Oct 2, 2020
t-johnson pushed a commit to t-johnson/Xamarin.Forms that referenced this issue Oct 16, 2020
Add option for custom renderers to control the execution mode of the WebView control
This is 'opt-in' as by default this commit will not change behavior of existing applciations.  to opt-in, people would need to set the ExecutionMode property in the constructor of their custom WebViewRenderer, like:

  public class MyWebViewRenderer : WebViewRenderer
  {
    public MyWebViewRenderer()
    {
      ExecutionMode = Windows.UI.Xaml.Controls.WebViewExecutionMode.SeparateProcess;
    }
  }

When set as 'SeparateProcess', the memory allocated by the WebView itself is all handled in a sub-process, which ensures the main process never crashes from running out of memory here from doing things like opening and closing youtube every 5 seconds.  This behavior will likely crash when the WebView is in-process due to the huge amounts of memory that is required by that website.
@samhouts samhouts removed this from the 5.0.0 milestone Nov 2, 2020
@samhouts samhouts added the in-progress This issue has an associated pull request that may resolve it! label Nov 2, 2020
@samhouts samhouts added this to In Progress in .NET MAUI Backlog Nov 2, 2020
@samhouts samhouts moved this from To do-High impact to In progress in UWP Ready For Work Nov 3, 2020
@rmarinho rmarinho moved this from To do to In Progress in vNext+1 (5.0.0) Nov 3, 2020
@PureWeen PureWeen added this to the 5.0.1 milestone Nov 5, 2020
@PureWeen PureWeen added this to To do in v5.0.1 via automation Nov 5, 2020
@PureWeen PureWeen removed this from In Progress in vNext+1 (5.0.0) Nov 5, 2020
@PureWeen PureWeen removed this from In Progress in .NET MAUI Backlog Nov 5, 2020
@PureWeen PureWeen modified the milestones: 5.0.1, 5.0.0 Nov 5, 2020
@PureWeen PureWeen added this to To do in vNext+1 (5.0.0) via automation Nov 5, 2020
@PureWeen PureWeen removed this from To do in v5.0.1 Nov 5, 2020
@PureWeen PureWeen added the blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. label Nov 5, 2020
@samhouts samhouts added this to In Progress in .NET MAUI Backlog Nov 5, 2020
@samhouts samhouts removed this from In Progress in .NET MAUI Backlog Nov 5, 2020
@wtheronjones
Copy link

I think I noticed this on a UWP project, perhaps having something to do w/ how the navigation system caches pages. (I could see them in "Microsoft Edge DevTools Preview")
I made it a point to navigate the webview to "about:blank" when leaving the screen containing the webview.

@rmarinho rmarinho moved this from To do to Blockers in vNext+1 (5.0.0) Nov 16, 2020
PureWeen pushed a commit to t-johnson/Xamarin.Forms that referenced this issue Nov 24, 2020
Add option for custom renderers to control the execution mode of the WebView control
This is 'opt-in' as by default this commit will not change behavior of existing applciations.  to opt-in, people would need to set the ExecutionMode property in the constructor of their custom WebViewRenderer, like:

  public class MyWebViewRenderer : WebViewRenderer
  {
    public MyWebViewRenderer()
    {
      ExecutionMode = Windows.UI.Xaml.Controls.WebViewExecutionMode.SeparateProcess;
    }
  }

When set as 'SeparateProcess', the memory allocated by the WebView itself is all handled in a sub-process, which ensures the main process never crashes from running out of memory here from doing things like opening and closing youtube every 5 seconds.  This behavior will likely crash when the WebView is in-process due to the huge amounts of memory that is required by that website.
@rmarinho rmarinho moved this from Blockers to In Progress in vNext+1 (5.0.0) Nov 27, 2020
UWP Ready For Work automation moved this from In progress to Done Dec 4, 2020
vNext+1 (5.0.0) automation moved this from In Progress to Done Dec 4, 2020
rmarinho pushed a commit that referenced this issue Dec 4, 2020
…12509)

* To address issue #4720:
Add option for custom renderers to control the execution mode of the WebView control
This is 'opt-in' as by default this commit will not change behavior of existing applciations.  to opt-in, people would need to set the ExecutionMode property in the constructor of their custom WebViewRenderer, like:

  public class MyWebViewRenderer : WebViewRenderer
  {
    public MyWebViewRenderer()
    {
      ExecutionMode = Windows.UI.Xaml.Controls.WebViewExecutionMode.SeparateProcess;
    }
  }

When set as 'SeparateProcess', the memory allocated by the WebView itself is all handled in a sub-process, which ensures the main process never crashes from running out of memory here from doing things like opening and closing youtube every 5 seconds.  This behavior will likely crash when the WebView is in-process due to the huge amounts of memory that is required by that website.

* tabs not spaces

* TemplatedItemsList: Ensure items are unhooked correctly when removing them, whether as individual removals or list resets.  Without this, the native cells do not actually get removed.
CellTableViewCell: use event PropertyChangedEventHandler instead of action (seems more standard)
TextCellRenderer: Correct the event delegate hookup
ListViewRenderer: Actually re-use the UITableViewCell when creating header sections, otherwise we endlessly create new ones, leaving the old ones alive through event handlers

* Revert "TemplatedItemsList: Ensure items are unhooked correctly when removing them, whether as individual removals or list resets.  Without this, the native cells do not actually get removed."

This reverts commit 7da9ffb.

* Make the custom renderer only apply to the test for this issue, as it seems the SeparateProcess affects access to cookies, breaking other tests

* Move additional classes inside test class to decrease namespace cluttering.

* - cleanup and rebase

* - add instructions

* - move WebViewExecutionMode to platform specific

* - fix up UI Test

* - clean up tests

* - fix tabs

* - fix formatting

* - fix teardown call

* - fix async ui test quirk on uwp

Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
pictos pushed a commit to pictos/Xamarin.Forms that referenced this issue Dec 30, 2020
…4720) (xamarin#12509)

* To address issue xamarin#4720:
Add option for custom renderers to control the execution mode of the WebView control
This is 'opt-in' as by default this commit will not change behavior of existing applciations.  to opt-in, people would need to set the ExecutionMode property in the constructor of their custom WebViewRenderer, like:

  public class MyWebViewRenderer : WebViewRenderer
  {
    public MyWebViewRenderer()
    {
      ExecutionMode = Windows.UI.Xaml.Controls.WebViewExecutionMode.SeparateProcess;
    }
  }

When set as 'SeparateProcess', the memory allocated by the WebView itself is all handled in a sub-process, which ensures the main process never crashes from running out of memory here from doing things like opening and closing youtube every 5 seconds.  This behavior will likely crash when the WebView is in-process due to the huge amounts of memory that is required by that website.

* tabs not spaces

* TemplatedItemsList: Ensure items are unhooked correctly when removing them, whether as individual removals or list resets.  Without this, the native cells do not actually get removed.
CellTableViewCell: use event PropertyChangedEventHandler instead of action (seems more standard)
TextCellRenderer: Correct the event delegate hookup
ListViewRenderer: Actually re-use the UITableViewCell when creating header sections, otherwise we endlessly create new ones, leaving the old ones alive through event handlers

* Revert "TemplatedItemsList: Ensure items are unhooked correctly when removing them, whether as individual removals or list resets.  Without this, the native cells do not actually get removed."

This reverts commit 7da9ffb.

* Make the custom renderer only apply to the test for this issue, as it seems the SeparateProcess affects access to cookies, breaking other tests

* Move additional classes inside test class to decrease namespace cluttering.

* - cleanup and rebase

* - add instructions

* - move WebViewExecutionMode to platform specific

* - fix up UI Test

* - clean up tests

* - fix tabs

* - fix formatting

* - fix teardown call

* - fix async ui test quirk on uwp

Co-authored-by: Shane Neuville <shneuvil@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/performance a/webview blocker Issue blocks next stable release. Prioritize fixing and reviewing this issue. e/5 🕔 5 in-progress This issue has an associated pull request that may resolve it! p/UWP partner/cat 😻 t/bug 🐛 up-for-grabs We welcome community contributions to any issue, but these might be a good place to start!
Projects
Development

Successfully merging a pull request may close this issue.

8 participants