New issue
Advanced search Search tips

Issue 640344 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Direct leaks when RenderingTest set data URL.

Project Member Reported by flackr@chromium.org, Aug 23 2016

Issue description

What steps will reproduce the problem?
(1) Create a RenderingTest which calls setBodyInnerHTML("<div style='background: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUg);'></div>"); (or use the attached LeakTest.cpp)
(2) Compile and run with ASAN leak detection (https://www.chromium.org/developers/testing/leaksanitizer)

See the attached file leak.txt for the leak stack traces.
 
leak.txt
37.7 KB View Download
LeakTest.cpp
480 bytes View Download
Components: -Blink Blink>Internals

Comment 2 by flackr@chromium.org, Aug 24 2016

Owner: flackr@chromium.org
Status: Started (was: Untriaged)
I think I may have found the leak. We create an ImageResource for the static image, and save it as the m_image Member of StyleFetchedImage but the destructor for the ImageResource is never called. The member pointer to it is cleared in StyleFetchedImage::dispose though.

Comment 3 by flackr@chromium.org, Aug 24 2016

I have a fix for the first leak of the SecurityOrigin object, but it is concerning that we don't seem to be freeing the RefPtr's held by the Resource when removing it: https://codereview.chromium.org/2278723002. I'd assume this means that we'd need to clear out the rest of its RefPtr's to ensure anything held onto is cleaned up.

Comment 4 by flackr@chromium.org, Aug 25 2016

Cc: haraken@chromium.org hirosh...@chromium.org
The second leak looks like the same cause. It's from the HashMap held onto by the Resource's ResourceRequest's HTTPHeaderMap.

The third and fourth are from the observer list, finished observer list and clients list leaking on the ImageResource/Resource object.

We really need to properly delete the Resource object when it's removed.

Comment 5 by flackr@chromium.org, Aug 25 2016

Looks like we just need to call memoryCache()->evictResources() when we're doing the RenderingTest TearDown so that these resources are freed. CL is updated: https://codereview.chromium.org/2278723002
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 26 2016

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

commit c90a6ca1d464e103448621a244ebb93e1f7d440c
Author: flackr <flackr@chromium.org>
Date: Fri Aug 26 12:33:45 2016

Evict resources from memory cache after running rendering test.

BUG= 640344 
TEST=Manual steps in bug.

Review-Url: https://codereview.chromium.org/2278723002
Cr-Commit-Position: refs/heads/master@{#414691}

[modify] https://crrev.com/c90a6ca1d464e103448621a244ebb93e1f7d440c/third_party/WebKit/Source/core/layout/LayoutTestHelper.cpp

Comment 7 by flackr@chromium.org, Aug 26 2016

Status: Fixed (was: Started)

Sign in to add a comment