New issue
Advanced search Search tips

Issue 739197 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug



Sign in to add a comment

SVG stroke-dasharray Transition - Memory Leak / Excessive Memory Usage

Reported by tom...@gmail.com, Jul 4 2017

Issue description

UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/59.0.3071.115 Safari/537.36

Steps to reproduce the problem:
*WARNING THIS MAY HANG YOUR SYSTEM*

1. Add an inline SVG with a strokable element

2. Add a CSS transition >= 1s for stroke-dasharray

3. Modify the stroke-dasharray attribute n times with n unique values at intervals slightly less than the transition duration.

What is the expected behavior?
Reasonable memory usage.

What went wrong?
On my system:

For n = 8
Chrome allocates ~4GiB of memory, sometimes recovers it, sometimes keeps it until closed.

For n = 16
Chrome allocates ~8GiB of memory (all), hangs the system, sometimes recovers.

For n = 32
Chrome allocates >8GiB of memory, hangs the system irrecoverably due to excessive swapping.

Did this work before? N/A 

Does this work in other browsers? Yes

Chrome version: 59.0.3071.115  Channel: stable
OS Version: 4.4.0-83
Flash Version: 

Relevant conditions / variables:

1. CSS transition must be enabled.

2. Transition duration needs to be around 1s or more (more does not increase usage but makes accumulation slower, and less may reduce usage due to not catching overlaps as reliably).

3. Interval between modifying stroke-dasharray attribute must be slightly less than transition duration in order that transitions overlap.

4. Segment list in stroke-dasharray seems to need to be around 8 or more long and new values must be unique (e.g alternating values with modulo doesn't cause the issue).
 
index.html
354 bytes View Download

Comment 1 by tom...@gmail.com, Jul 4 2017

I can also reproduce this on Windows 10... after spending 4 reboots and 2 hours updating the bastard thing... Now where's the UX bleach.

Comment 3 by tom...@gmail.com, Jul 4 2017

Also reproduced on Mac OS
Components: -Blink>CSS Blink>Animation
Labels: -OS-Linux -Hotlist-Interop Needs-Bisect Performance-Memory OS-All
Reproduced the test case in #2 on linux by opening the chrome task manager (shift-ESC), pressing "start" in the test case, and watching memory reach > 1gb for that tab.
Status: Untriaged (was: Unconfirmed)
Components: -Blink>Animation Blink>SVG
Status: Available (was: Untriaged)
This appears to be specific to SVG. If I swap the circle for a div element the memory usage doesn't explode to hundreds+ MB.

Example with SVG circle: https://jsfiddle.net/n0ugv4Lg/
Example with HTML div: https://jsfiddle.net/zLa6gshq/

Tested on Linux Chrome 60.

Components: -Blink>SVG Blink>Animation
I believe what's happening is that the repeating list behaviour of animations is creating unreasonably large stroke-dasharray values.

Here I add "console.log(getComputedStyle(target).strokeDasharray.length);":
https://jsfiddle.net/oscLr4tn/

In the console I see:
4
38
358
1798
19798
19798
257398
68
1048

This is because stroke-dasharray is being animated as a repeatable list of length values:
https://www.w3.org/TR/css3-transitions/#animtype-repeatable-list
Given two lists of different lengths the animated length is the "least common multiple of the lengths of the input lists".
All transitions that start after the first one will be animating between an animated value and a new value of different length. This causes the repeating to occur again and again, compounding on the previous repeated list.

This is both a spec issue in that such pathological scenarios are easy to encounter and an issue in that we hang the system with memory consumption attempting to execute this pathological scenario.

Looks like my test case that uses a div was incorrect, the value needs to be set on the inline style rather than the attribute.
Fixed div test case: https://jsfiddle.net/v5aagrve/
This also exhibits excessive memory usage.
Created a deterministic minimal repro: https://jsfiddle.net/y27hjkug/
Testing on Windows 10 with the example from #8, and setting n to 25, and transition-duration to 100s:

 - Chrome 59: allocates tons of memory, crashes with OOM
 - Firefox 54: allocates tons of memory, sometimes hangs, sometimes crashes
 - Edge 25: Runs the transition 10 times, and then stops. The page is still responsive

Chrome and FF appear to behave as per spec - until they crash! Edge is doing something else that prevents it from going crazy - possibly limiting the length of the stroke-dasharray property?


It turns out this is causing JS errors in Edge: "SCRIPT122: The data area passed to a system call is too small. _display (58,3)"

So they are pretty clearly placing limits on buffer sizes.

Capture.PNG
91.0 KB View Download

Comment 11 by tom...@gmail.com, Jul 5 2017

> Given two lists of different lengths the animated length is the "least common multiple of the lengths of the input lists".

That gave me an idea for a solution... Make the smaller list larger by adding zero length segments, this works for the test:

https://jsfiddle.net/drq1pb54/

This will of course change the behaviour, making segments grow or recede in between the ends of the pattern repeats. I don't think that would be a terrible thing, it looks like just as valid a transition to me. Is that level of behaviour defined in the spec?

That at least gives people a work around for the time being, just make sure all dasharrays are padded with zeros to the same length.
Labels: -Needs-Bisect
Owner: alancutter@chromium.org
Status: Started (was: Available)
Changing the existing behaviour is ultimately up to the spec authors and, given the nature of the web, fairly difficult to do. If you are interested in pursuing that then https://github.com/w3c/svgwg/issues is the place to start a discussion.

I have a workaround to limit how big this repeated list grows primarily to avoid crashing users machines without impacting existing web content that doesn't hit this extreme scenario.
https://chromium-review.googlesource.com/c/558899/
Project Member

Comment 13 by bugdroid1@chromium.org, Jul 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f9409bacee5b415ae2bc07c1a5d41bf9d5c23d4

commit 8f9409bacee5b415ae2bc07c1a5d41bf9d5c23d4
Author: Alan Cutter <alancutter@chromium.org>
Date: Thu Jul 06 02:14:53 2017

Limit the repeatability of repeatable list animations

The combination of CSS transition retargeting and properties that
animate as repeatable lists [0] can lead to scenarios where the number
of items in an animated value explodes to hundreds of thousands.

This is almost always unintentional and can be a source of system
lock ups for the user. This patch limits how much animations will
repeat two lists to be no more than 1000.

[0] https://www.w3.org/TR/css3-transitions/#animtype-repeatable-list

Bug:  739197 
Change-Id: Ib919b6af821f5d8e7cb98789c38ca568a70de52a
Reviewed-on: https://chromium-review.googlesource.com/558899
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Douglas Stockwell <dstockwell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#484444}
[add] https://crrev.com/8f9409bacee5b415ae2bc07c1a5d41bf9d5c23d4/third_party/WebKit/LayoutTests/animations/compounding-transitions-on-repeatable-list.html
[modify] https://crrev.com/8f9409bacee5b415ae2bc07c1a5d41bf9d5c23d4/third_party/WebKit/Source/core/animation/ListInterpolationFunctions.cpp

Status: Fixed (was: Started)

Sign in to add a comment