Heap Snapshot doesn't include all reachable objects
Reported by
b...@lucidchart.com,
Sep 15 2017
|
|||||||
Issue description
UserAgent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36
Steps to reproduce the problem:
Open the following HTML:
<html>
<body>
<script>
class Foo {
constructor() {
window.addEventListener('mousemove', (e) => {
this.x = e.pageX;
});
}
}
function bootstrap() {
const f = new Foo();
}
bootstrap();
</script>
</body>
</html>
Collect a heap snapshot. Search for instances of Foo.
What is the expected behavior?
Since the Foo instance is reachable, I expect to find it in the heap snapshot.
What went wrong?
It's not in the heap snapshot. If you put a breakpoint in the event listener, and hover over `this`, you'll see the instance of Foo is very much alive and well. But it's not in the heap snapshot.
Did this work before? Yes 60, possibly 61
Chrome version: 62 Channel: n/a
OS Version: Ubuntu 16.04
Flash Version:
A little embarrassing since I'm giving a conference talk on using the memory tools to find memory leaks on Monday, and this is the first thing I tried to set up as an example :-(
,
Sep 19 2017
+mlippautz: Is it possible that V8 objects kept alive due to wrapper tracing aren't captured in the heap snapshot?
,
Sep 20 2017
I think this is a problem with UnreachableObjectsFilter that doesn't do wrapper tracing. Essentially, the filter can be used for snapshots to filter out unreachable objects. The filter however, doesn't do ephemeral marking or wrapper tracing. I think this is in general flawed as the filter needs to mirror the GC behavior exactly and not just do best effort marking on its own as unmarked objects are not reported. This probably used to work when we just called MarkLiveObjects in the GC. This was however broken with incremental marking.
,
Sep 25 2017
Assigning to mlippautz; please feel free to reassign to the correct assignee on the V8 side.
,
Sep 27 2017
On a second look, this should work. Because the listener should still be reachable via a global handle and so the UnreachableObjectsFilter should not get rid of it. Any other thoughts from DevTools experts?
,
Sep 27 2017
A quick look revealed the following. I modified the example a bit to be able to find the Foo class:
<html>
<body>
<script>
class Foo {
constructor() {
window.addEventListener('mousemove', (e) => {
this.x = e.pageX;
});
}
}
function bootstrap() {
const f = new Foo();
return f;
}
zzz = bootstrap();
</script>
</body>
</html>
Now the Foo retaining tree looks like (see attachment 1)
The context @60389 seems to be missing retainers.
The DevTools on DevTools shows some warnings as well:
Heap snapshot: 7 nodes are unreachable from the root. Following nodes have only weak retainers:
@60389 weak retainers: (Global handles)@29.67
_buildPostOrderIndex @ Trie.js:1575
Will investigate further
,
Sep 28 2017
So to summarize what's going on. The Foo object is indeed reachable, but it is not reachable from the global object. So heap profiler treats it as V8 internal object and hides from the class list. You still can find it in the Containment view by exploring global handles. To address it I'm probably going to turn off the filtering as it started to produce lots of false negatives recently. Another problem however is that the retainment path for it would just be: Foo <- context <- anonymous function The rest is not visible as event listeners are beyond v8 heap. So the path does not give the user a hint on why the object is alive.
,
Sep 29 2017
Thanks a lot for debugging this! I'll move this over so you can decide whether followup actions are needed. > The rest is not visible as event listeners are beyond v8 heap. So the path does not give the user a hint on why the object is alive. That's an issue we've heard a few times recently. It looks like expanding the graph to the Blink side would be really useful.
,
Sep 29 2017
Actually, the (Foo <- context <- anonymous function) is usually sufficient hint to the user as to why it's still alive, since the dev tools let you click through to see the source code of the anonymous function. You probably see that it's being passed into addEventListener, and from there it becomes obvious. I think this is all the info we used to get in Chrome 60, but obviously I'd welcome more context if it's possible to get.
,
Oct 3 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/78b4c3503d857ed4285fc6bbd3eab9a4b6c80ec7 commit 78b4c3503d857ed4285fc6bbd3eab9a4b6c80ec7 Author: Alexei Filippov <alph@chromium.org> Date: Tue Oct 03 06:14:45 2017 DevTools: Do not filter out nodes unreachable from window in heap snapshot. There are user objects that are not reachable from root, e.g. event listeners. We want user be able to see them in the class list. BUG= 765666 Change-Id: I51e95cc9b5ae3d8530cc93d37781d1ee269a1ca7 Reviewed-on: https://chromium-review.googlesource.com/691187 Commit-Queue: Alexei Filippov <alph@chromium.org> Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org> Cr-Commit-Position: refs/heads/master@{#505963} [add] https://crrev.com/78b4c3503d857ed4285fc6bbd3eab9a4b6c80ec7/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot-event-listeners-expected.txt [add] https://crrev.com/78b4c3503d857ed4285fc6bbd3eab9a4b6c80ec7/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot-event-listeners.html [modify] https://crrev.com/78b4c3503d857ed4285fc6bbd3eab9a4b6c80ec7/third_party/WebKit/LayoutTests/http/tests/devtools/profiler/heap-snapshot.html [modify] https://crrev.com/78b4c3503d857ed4285fc6bbd3eab9a4b6c80ec7/third_party/WebKit/Source/devtools/front_end/heap_snapshot_worker/HeapSnapshot.js
,
Oct 5 2017
,
Oct 20 2017
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by kkaluri@chromium.org
, Sep 19 2017Labels: -Pri-2 hasbisect-per-revision Needs-Triage-M62 M-63 OS-Mac OS-Windows Pri-1
Owner: jbroman@chromium.org
Status: Assigned (was: Unconfirmed)