New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 609761 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

AdBlock memory leak in M51

Project Member Reported by u...@chromium.org, May 6 2016

Issue description

The V8 range for the chrome range: https://chromium.googlesource.com/v8/v8/+log/5.1.281.8..5.1.294.3

In this range we "fixed" the old generation limit
  https://chromium.googlesource.com/v8/v8/+/d16c3825fbd91997552b1c9251ec5a436fe80e8a

hpayer@: Maybe any unconsidered side effects?
Project Member

Comment 2 by sheriffbot@chromium.org, May 6 2016

Labels: M-51 Fracas
Users experienced this crash on the following builds:

Win Dev 52.0.2723.3 -  2.45 CPM, 33 reports, 30 clients (signature Out of Memory (v8))
Win Canary 52.0.2726.0 -  3.73 CPM, 21 reports, 19 clients (signature Out of Memory (v8))
Mac Dev 52.0.2723.2 -  12.25 CPM, 5 reports, 5 clients (signature Out of Memory (v8))
Mac Canary 52.0.2725.0 -  15.41 CPM, 70 reports, 66 clients (signature Out of Memory (v8))
Linux Dev 52.0.2716.0 -  25.29 CPM, 101 reports, 62 clients (signature Out of Memory (v8))
Linux Beta 51.0.2704.36 -  8.21 CPM, 10 reports, 8 clients (signature Out of Memory (v8))
Android Dev 52.0.2723.0 -  3.06 CPM, 44 reports, 28 clients (signature Out of Memory (v8))

If this update was incorrect, please add "Fracas-Wrong" label to prevent future updates.

- Go/Fracas

Comment 3 by u...@chromium.org, May 10 2016

Cc: hablich@chromium.org

Comment 4 by u...@chromium.org, May 10 2016

Cc: u...@chromium.org
Owner: cbruni@chromium.org
Regression range from #1 was not precise.

I bisected memory leak manually to https://codereview.chromium.org/1707743002

Repro steps:

1. Install AdBlock extension (not AdBlockPlus).
2. Open http://techcrunch.tumblr.com
3. Duplicate the tab 7 times.
4. Open task manager, add JavaScript Memory column.

Expected: the memory usage is at 150MB.
What happens instead: memory usage is at 300MB and grows if tabs are refreshed.

Camillo, could you please take a look? The memory leak is massive and leading to many OOM crashes in AdBlock extension process.

Comment 5 by u...@chromium.org, May 10 2016

Summary: AdBlock memory leak in M51 (was: Extension process V8 OOM spike in M52)

Comment 6 by ananthak@google.com, May 10 2016

Labels: ReleaseBlock-Dev
Please fix this ASAP, so we can test the fix in Canary, dev, and merge it to Beta. Marking this as dev blocker for tracking purposes. 

Comment 7 by cbruni@chromium.org, May 11 2016

Status: Started (was: Assigned)
The crash first appeared in chrome version 51.0.2662.0. This crash hots the latest builds as below

52.0.2730.0	0.16%	13	canary
52.0.2729.3	0.05%	4       dev
51.0.2704.36	0.39%	31	beta

Seeing a spike between the versions 52.0.2714.0 and 52.0.2715.0.

Comment 10 by u...@chromium.org, May 11 2016

Cc: cbruni@chromium.org
Owner: u...@chromium.org
Update on what Camillo and I found out today:

1. PrependElementIndicesImpl allocates large array of 91807 elements and right trims it to 16875.
2. Each right trim adds error to our live object size counter. The counter is larger than the actual live object size.
3. Eventually the counter reaches the max heap limit and we crash with OOM, even though the actual memory usage is low.

I am looking into why the counter gets confused by right trimming.

Comment 11 by u...@chromium.org, May 11 2016

Found the bug: LargeObjectSpace::FreeUnmarkedObjects decrements the counter by the current size of the object instead of original size.
Just to confirm, this does not need to be merged to 51, right?

Comment 14 by u...@chromium.org, May 12 2016

Yes, I will merge after canary coverage.
Labels: merge-review-5.1

Comment 16 by u...@chromium.org, May 13 2016

Labels: Merge-Request-51
Canary looks good! V8 OOM for extention process is down.

Comment 17 by tin...@google.com, May 13 2016

Labels: -Merge-Request-51 Merge-Approved-51 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M51 (branch: 2704)
Project Member

Comment 18 by bugdroid1@chromium.org, May 13 2016

Labels: merge-merged-5.1
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/11fc4de1ed401927a556db37d5fa8b0fb84ba8b5

commit 11fc4de1ed401927a556db37d5fa8b0fb84ba8b5
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Fri May 13 16:20:37 2016

Version 5.1.281.35 (cherry-pick)

Merged 12fa3fff651181098956fa46bac7c3ac1d1424ab

Fix live bytes counter in large object space after right trimming.

BUG= chromium:609761 
LOG=N
R=vogelheim@chromium.org

Review URL: https://codereview.chromium.org/1977883002 .

Cr-Commit-Position: refs/branch-heads/5.1@{#41}
Cr-Branched-From: 167dc63b4c9a1d0f0fe1b19af93644ac9a561e83-refs/heads/5.1.281@{#1}
Cr-Branched-From: 03953f52bd4a184983a551927c406be6489ef89b-refs/heads/master@{#35282}

[modify] https://crrev.com/11fc4de1ed401927a556db37d5fa8b0fb84ba8b5/include/v8-version.h
[modify] https://crrev.com/11fc4de1ed401927a556db37d5fa8b0fb84ba8b5/src/heap/heap.cc
[modify] https://crrev.com/11fc4de1ed401927a556db37d5fa8b0fb84ba8b5/src/heap/spaces.h
[modify] https://crrev.com/11fc4de1ed401927a556db37d5fa8b0fb84ba8b5/test/cctest/heap/test-heap.cc

Comment 19 by u...@chromium.org, May 13 2016

Status: Fixed (was: Started)
Labels: -Merge-Approved-51 -merge-review-5.1
Per comment #18, this is already merged to M51. So removing "Merge-Approved-51" & "merge-review-5.1" labels. 
Project Member

Comment 23 by bugdroid1@chromium.org, Nov 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/14c6a651d1c04c90379299e577dd8e187967f5a9

commit 14c6a651d1c04c90379299e577dd8e187967f5a9
Author: cbruni <cbruni@chromium.org>
Date: Tue Nov 15 18:30:35 2016

[elements] Precisely estimate elements size as last resort

In case of an allocation failure in for-in over holey elements, use precise
number of elements to allocate a smaller buffer for the collected indices.

Drive-by-fix: make is_the_hole accept the isolate for faster checks.

BUG= chromium:609761 

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

[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/compiler/js-create-lowering.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/compiler/js-global-object-specialization.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/elements.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/elements.h
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/factory.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/factory.h
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/lookup.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/objects-inl.h
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/objects.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/objects.h
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/runtime/runtime-scopes.cc

Project Member

Comment 24 by bugdroid1@chromium.org, Nov 18 2016

Labels: merge-merged-5.6
The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/14c6a651d1c04c90379299e577dd8e187967f5a9

commit 14c6a651d1c04c90379299e577dd8e187967f5a9
Author: cbruni <cbruni@chromium.org>
Date: Tue Nov 15 18:30:35 2016

[elements] Precisely estimate elements size as last resort

In case of an allocation failure in for-in over holey elements, use precise
number of elements to allocate a smaller buffer for the collected indices.

Drive-by-fix: make is_the_hole accept the isolate for faster checks.

BUG= chromium:609761 

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

[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/compiler/js-create-lowering.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/compiler/js-global-object-specialization.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/elements.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/elements.h
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/factory.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/factory.h
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/lookup.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/objects-inl.h
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/objects.cc
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/objects.h
[modify] https://crrev.com/14c6a651d1c04c90379299e577dd8e187967f5a9/src/runtime/runtime-scopes.cc

Sign in to add a comment