New issue
Advanced search Search tips

Issue 751310 link

Starred by 2 users
Status: Assigned
Owner:
etienneb@chromium.org
Cc:
kouhei@chromium.org
haraken@chromium.org
erikc...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Potential memory leak with DocumentFragment / Serialisation

Project Member Reported by etienneb@chromium.org, Aug 1 2017 
I keep running a canary browser with native heap profiling (memory-infra) activated and look for remaining objects alive after a long period (5 days).

The following stackframe is indicating a potential memory leak.

      "blink::(anonymous namespace)::ReportAllocation(void*, unsigned long, char const*)",
      "WTF::StringImpl::CreateUninitialized(unsigned int, unsigned char*&)",
      "WTF::String::Make8BitFrom16BitSource(unsigned short const*, unsigned long)",
      "blink::AtomicHTMLToken::AtomicHTMLToken(blink::HTMLToken&)",
      "blink::HTMLDocumentParser::PumpTokenizer()",
      "blink::HTMLDocumentParser::Append(WTF::String const&)",
      "blink::HTMLDocumentParser::ParseDocumentFragment(WTF::String const&, blink::DocumentFragment*, blink::Element*, blink::ParserContentPolicy)",
      "blink::CreateFragmentForInnerOuterHTML(WTF::String const&, blink::Element*, blink::ParserContentPolicy, char const*, blink::ExceptionState&)",
      "blink::Element::setInnerHTML(WTF::String const&, blink::ExceptionState&)",
      "blink::V8Element::innerHTMLAttributeSetterCallback(v8::FunctionCallbackInfo<v8::Value> const&)",


The memory allocations related to tokens got leaked when the HTML is invalid.
see:
  https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/editing/serializers/Serialization.cpp?type=cs&l=595

They got leaked when reaching this code:
 if (!was_valid) {
    exception_state.ThrowDOMException(


We can trigger the memory leak by loading the following HTML page:
========================================================================

<html>
<body>
counting: <div id="count"> ??? </div>
<div id="bug"> ... </div>
<script>

// Making a useless HUGE string (to be leaked.)
var x = "<a>";
var iterations = 14;
for (var i = 0; i < iterations; i++) {
  x += x+x;
}

  var count = 0;
  setInterval(()=> {
      document.getElementById("count").innerHTML = count;
      document.getElementById("bug").innerHTML = x + "<a href=<";  // Invalid HTML
      count++;
    }, 1000);
</script>

</body>
</html>

Comment 1 by erikc...@chromium.org, Aug 1 2017

Cc: -etienneb@chromium.org haraken@chromium.org
Owner: etienneb@chromium.org
Status: Assigned (was: Untriaged)
Nice find! 

I believe Etienne is planning on putting together a fix tomorrow. haraken@, who would be a good reviewer?

Comment 3 by haraken@chromium.org, Aug 1 2017

Cc: kouhei@chromium.org
kouhei@ and/or me.

Comment 4 by kouhei@chromium.org, Aug 1 2017

Components: -Blink Blink>HTML>Parser
Yes please add me on the review

Comment 6 by etienneb@chromium.org, Aug 2 2017 

As dcheng@ pointed out, these allocations are garbage collected.
I didn't point to the right source of the memory leak.

But, there is a memory leak for sure. (see attachment)
memory_leak.png
10.2 KB View Download

Comment 7 by etienneb@chromium.org, Aug 2 2017 

The repro code is not representative on the above stackframe.
The memory got bloated because the string seems too big.
 >> var iterations = 14;

By moving it down to it seems fine.
 >> var iterations = 10;

I'm gonna continue to dig into how to get the leaked stackframe with a repro case.
►

Sign in to add a comment