1627739 - With WebRender enabled, off-screen GIFs cause memory leak

Status: VERIFIED FIXED

(Core :: Graphics: WebRender, defect, P2)

Description

[nyanpasu64]

Attachment: about support clean.txt

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:74.0) Gecko/20100101 Firefox/74.0

Steps to reproduce:

• Optionally create a clean profile. Optionally install uBlock Origin.
• Enable WebRender (about:config, gfx.webrender.all = true).
• Visit https://stackoverflow.com/questions/47455756/how-to-trigger-documentation-popup-in-vscode/
• If you scroll down, you'll see a large animated GIF. Ensure the GIF is not visible (don't scroll down).
• Ensure the tab is focused and Firefox is not minimized. The bug still happens if Firefox is covered by other windows.

Actual results:

• Looking at Task Manager, the tab's content process memory usage rises rapidly (100-200 MB/s).
• Every time you interact with the page (focus/unfocus the window, scroll the page, move your mouse), the memory usage resets to normal.
• If the GIF is visible, memory does not leak.
• If you do nothing and let memory leak, Firefox's memory usage can go into the gigabytes. I've seen 3-10GB, to the point that other applications were failing to allocate memory.

In Task Manager, working set rises, but active private working set does not.

I took a memory dump of the content process when 3 gigabytes was allocated. By loading the memory dump into GIMP as "Raw image data", I saw that much of the memory was taken up by RGBA frames of the GIF (2732 pixels wide)

Expected results:

Memory usage doesn't rise without limit.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Comment 2

[Timothy Nikkel (:tnikkel)]

regression range:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=1330041fa8973ac2a56f074a7c1eee9bb1cff151&tochange=93ccc760c4dd69f63d6ac171ace357b0dd31412a

probably bug 1599656

Lack of composites causing the image frames to build up in a queue?

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

Seems to be a bug in recycling. If I turn off image.animated.decode-on-demand.recycle, the memory consumption is stable (well it is always freeing / reallocating buffers but doesn't increase above a reasonable ceiling).

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Ultimately what appears to happen is:

1. FrameAnimator::GetCompositedFrame is called, we advance the frame
2. Eventually RenderRootStateManager::FlushAsyncResourceUpdates is called to flush the changes
3. In WebRenderBridgeParent::RecvUpdateResources we now will wait for a Checkpoint::FrameTexturesUpdated notification to release the old buffer for recycling
4. The notification never comes, so when we need to advance the animation again, we eventually run out of buffers and allocate a new one; this continues forever

The notification never comes because it decided the frame did not need to be updated. I think when we issue a layer transaction we hit this path:

https://searchfox.org/mozilla-central/rev/2cd2d511c0d94a34fb7fa3b746f54170ee759e35/gfx/wr/webrender/src/render_backend.rs#1677

which causes us not to re-render the next frame. As a result, we never actually flush the pending texture updates (themselves which appear to be empty), and we are always defering the future notifications.

Comment 5

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1627739 - Do not delay FrameTexturesUpdated notifications for empty updates.

The FrameTexturesUpdated event is intended to be issued when any
necessary texture cache updates have been completed for the current
frame. Originally we would simply check if the pending_texture_updates
vector in Renderer is empty. However the updates themselves could be
nops, and not trigger a timely frame render to occur, causing the
notifications to be delayed indefinitely. Now we track if there are
actually any pending specifically texture cache updates and only defer
the notification if true.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e0fdd643f2e
Do not delay FrameTexturesUpdated notifications for empty updates. r=kvark

Comment 7

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/3e0fdd643f2e

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:aosmond, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 9

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9140275 [details]
Bug 1627739 - Do not delay FrameTexturesUpdated notifications for empty updates.

Beta/Release Uplift Approval Request

• User impact if declined: May see memory increase when animated images are in the visible tab/window, but are in the current viewport when using WebRender.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): It is a minor tweak to how we issue WebRender notifications, we send them a little sooner than we did before, and only when the state could not possibly have changed.
• String changes made/needed:

Comment 10

[Ryan VanderMeulen (PTO, back July 27)]

Comment on attachment 9140275 [details]
Bug 1627739 - Do not delay FrameTexturesUpdated notifications for empty updates.

Fixes a possible memory leak in WebRender. Approved for 76.0b7.

Comment 11

[Ryan VanderMeulen (PTO, back July 27)]

https://hg.mozilla.org/releases/mozilla-beta/rev/22851affc3b9

Comment 12

[Daniel Bodea [:danibodea]]

I can't seem to be able to reproduce on my system. I have attempted the steps in comment 0 in Firefox Release v74.0 and Firefox Release v75.0. Checked both the about:performance tab and the Windows Task Manager and there was no rapid increase of used memory.

Does this issue still reproduce for you? Can you reproduce in the latest Release Version of Firefox, v75.0?
If yes, could you attempt to reproduce is in the latest Beta and Nightly versions. Hopefully, it is now fixed in those versions.
Thank you for your contribution!

Comment 13

[nyanpasu64]

Yes I still get the issue in Firefox 75.0 on Windows. I killed Firefox after 1.6GB of allocations.

The bug also occurs on a Linux machine with 8GB of RAM. The memory leak (ksysguard Shared Mem, not Memory) sometimes resolves after 1GB, and sometimes continues to 4GB at which point the tab crashes (from OOM I assume).

The issue does not occur on Firefox Developer 76.0b7 (64-bit) on Windows. I did not test Developer or Nightly on Linux.

Comment 14

[Daniel Bodea [:danibodea]]

Verifying firefox76 based on the previous comment. Thank you very much, reporter.
It would be awesome if you could also confirm that it doesn't occur in the latest Nightly, eighter, so we can close this issue properly:
http://archive.mozilla.org/pub/firefox/nightly/latest-mozilla-central/

Comment 15

[nyanpasu64]

I can't seem to be able to reproduce on my system.

Don't know why. Opening Stack Overflow in a private window, the dark mode banner is initially animated, but the memory leak eventually happens anyway.

Anyway no leak in Firefox Nightly 77.0a1 (2020-04-22) (64-bit), on Windows 10 x64. Seems fixed there too.

Comment 16

[Daniel Bodea [:danibodea]]

Much appreciated!