1531333 - Rapid OOM on inclusive.tki.org.nz

Status: VERIFIED FIXED

(Core :: SVG, defect, P2)

Description

[BWK]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

Steps to reproduce:

Happens randomly with random websites, has never happened up until two weeks ago and occurred three times since then. After the first time the system was reinstalled from scratch, with /tmp moved to its own partition after the first crash exhausted all free tmp space causing other apps to crash as well. Occurred twice since reinstalling but able to recover with /tmp unaffected.

Actual results:

You'll try to open some web site (not the same one each time). The web site will fail to load and the system will max out on memory use and start swapping to disk (swap partition). A process called Web Content will be visible in the process monitor and will be clearly rapidly increasing in resource usage and continue to do so until all system resources are exhausted at which point Firefox crashes. In this case it consumed up to 100 GB of virtual memory (system has 32 GB physical and 100 GB swap). After crashing the system went down to 3 GB of physical memory and 24 GB of swap in use for the other apps. The 3rd time it happened, FF was killed by the user before crashing. The first step to close the tab that was trying to load the website supposedly causing the problem has no impact as Web Content continues to gobble more resources. Only possiible to stop this runaway by closing the Firefox window. Then the Web Content process disappears and the system goes back to normal.

Expected results:

When Firefox is restarted after crashing and attempting to access the same web sites as were running before crash there is only a small resource usage overall and no runaway occurring. Since the web site that caused the crash now loads normally it is not possible to prove there is an issue with that particular website or trace what is going on in the system.

Comment 1

[BWK]

OK
I have pinned down a website that consistently crashes Firefox Developer Edition.

Address: http://www.inclusive.tki.org.nz which is a website published by the Government of New Zealand.

This website can be guaranteed in testing so far to cause Firefox Developer's Web Content process to run away and crash after exhausting all the system's virtual memory every single time.

Does not crash Firefox ESR, only Developer.

Tested over multiple different computers and configurations with same results every time.

Sorry I have to rush this before my browser crashes again.

Comment 2

[Marcia Knous [:marcia]]

BWK: Do you get a crash report in about:crashes? If so, can you please paste it here? Thanks.

Comment 3

[BWK]

Crash report 1472b011-c379-150a-2c62-8a0c12d2e110 was submitted.

Sometimes the browser will crash, sometimes it won't. In this instance, the Web Content process crashed at virtual memory usage of 100 GiB, but the browser stayed open, so possibly it only crashed the tab.

Comment 4

[Marcia Knous [:marcia]]

(In reply to BWK from comment #3)

Crash report 1472b011-c379-150a-2c62-8a0c12d2e110 was submitted.

Sometimes the browser will crash, sometimes it won't. In this instance, the Web Content process crashed at virtual memory usage of 100 GiB, but the browser stayed open, so possibly it only crashed the tab.

I cannot find that report (bp-1472b011-c379-150a-2c62-8a0c12d2e110) - can you paste the entire sequence with the first two characters? Thanks.

Comment 5

[BWK]

https://crash-stats.mozilla.org/report/index/a7e83f4c-b338-4963-90aa-28df50190301

Comment 6

[André Bargull [:anba]]

Stack trace in crash report shows recursive calls to nsNodeUtils::CloneAndAdopt, therefore moving to DOM: Core.

Comment 7

[Olli Pettay [:smaug][[e-mail]]]

That sounds like bug in the web page, but I can't now reproduce.

If anyone can reproduce, regression range would be really good.

Comment 8

[Andrew McCreight [:mccr8]]

(In reply to André Bargull [:anba] from comment #6)

Stack trace in crash report shows recursive calls to nsNodeUtils::CloneAndAdopt, therefore moving to DOM: Core.

This is an OOM crash on an allocation of 250 bytes, so the stack is not relevant.

What we really need here is a memory report. There might also be one to dig out of the crash report.

Comment 9

[Alice0775 White]

I can reproduce the issue on Nightly68.0a1 Windows10.

STR

1. Open https://www.inclusive.tki.org.nz/

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=38e729d91ed2873b708bd4800b01b8433ffceabd&tochange=453d0a08f00c125ff6cc3905265e76efe641524e

Regressed by: Bug 1450250

Comment 10

[Andrew McCreight [:mccr8]]

Thanks for the regression window!

Emilio, can you take a look?

Comment 11

[Alice0775 White]

And also this https://www.inclusive.tki.org.nz/about-this-website/ .

Comment 12

[Alice0775 White]

Culprit seems to be https://www.inclusive.tki.org.nz/themes/inclusive/src/img/icons/svg-sprite.svg

Comment 13

[Andrew McCreight [:mccr8]]

[Tracking Requested - why for this release]: This is a really bad OOM. The regression window points to a bug that landed in 63, so maybe this website changed recently. It isn't clear how widespread this is, given that this is apparently the first report we've gotten, but it still seems like we should fix it if we can given how bad it is.

Comment 14

[Emilio Cobos Álvarez (:emilio)]

Attachment: crashes.tar.gz

In the tarball (since bugzilla insists in auto-previewing the attachment while I'm attaching it), there are two svgs.

The problematic one from the URL Alice has mentioned (thanks Alice!), and one with three more lines of code which also crashes Firefox 63 and before:

<style>
  symbol { display: block }
</style>

This is a bug in our cycle detection for SVG which causes it to blow up quite a bit. It just wasn't doing it before because it was using NAC, and NAC doesn't get generated for undisplayed elements like the <use> inside the <symbols> elements.

This is a bogus svg fwiw, it looks like some tool has generated it quite poorly. Still we should fix this.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1531333 - Fix <svg:use> cycle detection. r=heycam,longsonr,smaug

With the current code we'll eventually detect the cycle, but will take much
more, creating many shadow trees unnecessarily. Take for example the following:

<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="133" height="232774">
<style>
symbol { display: block }
</style>
<symbol id="svg-sprite" viewBox="0 0 133 230866">
<title>svg-sprite</title>
<symbol id="svg-sprite" viewBox="0 0 133 230866">
<title>svg-sprite</title>
<use xlink:href="#svg-sprite" width="500" height="500" />
</symbol>
<use xlink:href="#svg-sprite" y="1601" width="133" height="228958" />
</symbol>
<use xlink:href="#svg-sprite" y="1601" width="133" height="230866" />
</svg>

Before this patch, we'd create an svg use element subtree for #svg-sprite. That
subtree will contain two other <use> elements, one under the <symbol>, one not
under it.

Both point to #svg-sprite, but we fail to detect we're an ancestor since the
element #svg-sprite we're looking at is the clone of the #svg-sprite element.

Thus we need to take a look at mOriginal instead (which is the <use> element
under #svg-sprite) rather than at the clone.

Yeah, I had to draw the trees, it's messy :)

Blink and WebKit do something slightly different (they check the element id
directly1). That's not 100% correct, since you can have multiple elements with
the same ID.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/88b6e96d8215
Fix <svg:use> cycle detection. r=longsonr

Comment 18

[BWK]

So far as I know this is a relatively new website.

I can't crash regular firefox when loading this site. Only happens with Developer.

Comment 19

[Stefan Hindli [:stefan_hindli]]

https://hg.mozilla.org/mozilla-central/rev/88b6e96d8215

Comment 20

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9052983 [details]
Bug 1531333 - Fix <svg:use> cycle detection. r=heycam,longsonr,smaug

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: 1450250 for this specific test-case, but it was a pre-existing issue, see bug 743636
• User impact if declined: OOM
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: visit one of the pages listed in the bug and see them load to completion :)
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Low-risk fix to our cyclic <svg:use> detection.
• String changes made/needed: none

Comment 21

[Pascal Chevrel:pascalc]

Comment on attachment 9052983 [details]
Bug 1531333 - Fix <svg:use> cycle detection. r=heycam,longsonr,smaug

Fix for a bad memory leak in our SVG support causing an OOM crash. Small patch with tests already on Nightly. Uplift approved for 67 beta 5, thanks.

Comment 22

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/ae13953b5005

Comment 23

[Rares Doghi, Desktop QA]

Hi, I have retested this issue in Firefox Beta 67.0b5 as well as the latest nightly 68.0a1 (2019-03-26) using Windows 10 as well as Ubuntu 18.04 and the issue no longer occurs. I will mark this issue accordingly