New issue
Advanced search Search tips

Issue 796884 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Regression



Sign in to add a comment

blink::Animation objects leak in background tab

Reported by yusj....@gmail.com, Dec 21 2017

Issue description

Steps to reproduce the problem:
1. open the attach file animation_leak.html.
2. open another new blank tab, thus the tab with 'animation_leak.html' is running in background.
3. wait for 10 hours or longer.
4. blink::Animation objects will reach as 36,000 or more.
5. wait more longer, the tab with animation_leak.html will crashes, the crash call stack is like below:

Thread Name: 'Chrome_InProcRe'
pid: 9795, tid: 15759  >>> com.UCMobile <<<
signal 4 (SIGILL), code 1 (ILL_ILLOPC), fault addr 85cf6696
  r0 00000000  r1 0000000c  r2 31b87a0d  r3 31b87a0d
  r4 00140000  r5 00000000  r6 00000000  r7 0000000a
  r8 10008038  r9 7e40821c  10 7e4073b0  fp 7e3ff07c
  ip 86c962c8  sp 7e3fed70  lr 85d15801  pc 85cf6696  cpsr 680d0030
#00 pc 0x00bb4696 blinkGCOutOfMemory at PageMemory.cpp:57
#01 pc 0x00bd37ff blink::PageMemoryRegion::allocate(unsigned int, unsigned int, blink::RegionTree*) at PageMemory.cpp:75
#02 pc 0x00bd28d3 blink::PageMemoryRegion::allocateNormalPages(blink::RegionTree*) at PageMemory.h:78
     (inlined by) blink::NormalPageArena::allocatePage() at HeapPage.cpp:639
#03 pc 0x00bd2c03 blink::NormalPageArena::outOfLineAllocate(unsigned int, unsigned int) at HeapPage.cpp:958
#04 pc 0x011012a7 WTF::VectorBufferBase<blink::Member<blink::Animation>, false, blink::HeapAllocator>::allocateBuffer(unsigned int) at Vector.h:343
     (inlined by) WTF::Vector<blink::Member<blink::Animation>, 0u, blink::HeapAllocator>::reserveInitialCapacity(unsigned int) at Vector.h:1324
     (inlined by) blink::AnimationTimeline::serviceAnimations(blink::TimingUpdateReason) at AnimationTimeline.cpp:129
#05 pc 0x011cde3b blink::Document::updateStyleAndLayoutTree() at Document.cpp:2002
#06 pc 0x011ce049 blink::Document::updateStyleAndLayoutTreeIgnorePendingStylesheets() at Document.cpp:2292
#07 pc 0x011ce071 blink::Document::updateStyleAndLayoutIgnorePendingStylesheets(blink::Document::RunPostLayoutTasks) at Document.cpp:2297
#08 pc 0x012ca169 blink::HTMLElement::unclosedOffsetParent() at HTMLElement.cpp:1209 (discriminator 1)
#09 pc 0x012ca201 blink::HTMLElement::offsetWidthForBinding() at HTMLElement.cpp:1188
#10 pc 0x01089769 offsetWidthAttributeGetter at V8HTMLElement.cpp:590
     (inlined by) blink::HTMLElementV8Internal::offsetWidthAttributeGetterCallback(v8::FunctionCallbackInfo<v8::Value> const&) at V8HTMLElement.cpp:594
#11 pc 0x4859a006 (null)

To accelerate the leak, we can disable background throttling by modify code as below:
in src/third_party/WebKit/Source/platform/scheduler/renderer/web_view_scheduler_impl.cc
void WebViewSchedulerImpl::UpdateBackgroundThrottlingState() {
  return;   // <<<<<<<<<<<<<<<<<<<<<<<<< add return here
  for (WebFrameSchedulerImpl* frame_scheduler : frame_schedulers_)
    frame_scheduler->SetPageVisible(page_visible_);
  UpdateBackgroundBudgetPoolThrottlingState();
}

Thus the javascript timer can run more times rather than 1 time per second.

What is the expected behavior?
No matter how long the tab with 'animation_leak.html' is running in background, the num of blink::Animation objects should not increase without limits, and not crash with OOM also.

What went wrong?
Animation in non-visible webview is not processed, and cached in AnimationTimeline::m_animationsNeedingUpdate (newest code is in DocumentTimeline::animations_needing_update_). But the HeapHashSet size has not limits, so blink::Animiation objects will grow indefinitely.

Crashed report ID: 

How much crashed? Just one tab

Is it a problem with a plugin? No 

Did this work before? Yes 

Chrome version: 61.0.3163.79  Channel: stable
OS Version: any
Flash Version:
 
animation_leak.html
10.0 KB View Download
Components: Blink>Animation
Labels: Needs-Bisect
Status: Untriaged (was: Unconfirmed)
I confirmed that the pending animations vector is growing by adding the following to DocumentTimeline::ServiceAnimations


  if (animations_needing_update_.size() > 10) {
    LOG(WARNING) << "animations_needing_update_.size() ("
                   << animations_needing_update_.size() << ")";
  }


This may not be practical to bisect.

Blockedon: 706281
Status: Available (was: Untriaged)
Blockedon: -706281
Labels: -Stability-Crash
Status: Assi (was: Available)
I do not understand why this was blocked on the crash server meta-bug, it was a report from an end-user.

Majid, can you confirm if this still reproduces and attempt a bisect if so?
Cc: smcgruer@chromium.org
Owner: majidvp@chromium.org
Status: Assigned (was: Assi)

Comment 6 by sunxd@chromium.org, May 25 2018

Labels: Stability-Crash
Is Stability-crash the right label? This is a memory leak
Status: Started (was: Assigned)
I have started looking into this issue and have created a small self-contained repro based on the initial report which is attached. 

## Current Hypothesis

When page is in background, CC throttle main frames. This means we never run DocumentTimeline::ServiceAnimations which is responsible for removing finished animations.

Meanwhile there is recurring timer event (which is not throttled) that starts a new transition and also triggers a style recalc (e.g., updating page content). As a result we will end up in CSSAnimations::MaybeApplyPendingUpdate which creates and plays on the new animation that represents the transition.

The combination of these two means that we accumulate, new animations without actually ever playing them.


## Solution?

I am not sure yet but one simple solution is to avoid creating new transitions when page is throttled.

 
animation_leak_minimum.html
426 bytes View Download
Cc: flackr@chromium.org chrishtr@chromium.org
+ chrishtr@, flackr@


chrishtr@: Do you know if something like this has ever come up before? Basically throttling rendering prevents frame based document lifecycles but document lifecycle may still be triggers from script. 
This seems to be problematic for css transitions where they may be started but are not progressed. This leads to accumulated *zombie* transitions causing memory leak. I wonder if my idea in #8 to not
start the transition when rendering is throttled makes sense here or perhaps there are better solutions here.

 
Not starting transitions could cause script based on transition events to fail. I think it would be better to finish pending animations which haven't started within their animation duration during MaybeApplyPendingUpdate.

Sign in to add a comment