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

Issue 825741 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Layout Test external/wpt/IndexedDB/wasm-module-value.html is leaking

Project Member Reported by battre@chromium.org, Mar 26 2018

Issue description

The following layout test is failing on WebKit Linux Trusty Leak

* external/wpt/IndexedDB/wasm-module-value.html
* external/wpt/wasm/wasm_local_iframe_test.html
* fast/workers/worker-shared-asm-buffer.html

Probable cause:

00:49:58.697 23783 worker/5 external/wpt/IndexedDB/wasm-module-value.html leaked
00:49:58.703 23634 [2452/6535] external/wpt/IndexedDB/wasm-module-value.html failed unexpectedly (leak detected: ({"numberOfLiveDocuments":[1,2],"numberOfLiveNodes":[4,33],"numberOfLivePausableObjects":[2,9],"numberOfLiveResourceFetchers":[1,2],"numberOfLiveResources":[0,3]}))
00:49:58.702 23783 worker/5 external/wpt/IndexedDB/wasm-module-value.html failed:
00:49:58.702 23783 worker/5  leak detected: ({"numberOfLiveDocuments":[1,2],"numberOfLiveNodes":[4,33],"numberOfLivePausableObjects":[2,9],"numberOfLiveResourceFetchers":[1,2],"numberOfLiveResources":[0,3]})

00:52:14.573 18825 worker/3 external/wpt/wasm/wasm_local_iframe_test.html leaked
00:52:14.575 18683 [4100/6516] external/wpt/wasm/wasm_local_iframe_test.html failed unexpectedly (leak detected: ({"numberOfLiveDocuments":[1,3],"numberOfLiveFrames":[1,2],"numberOfLiveNodes":[4,27],"numberOfLivePausableObjects":[2,5],"numberOfLiveResourceFetchers":[1,3],"numberOfLiveResources":[0,3]}), asserts failed)
00:52:14.574 18825 worker/3 external/wpt/wasm/wasm_local_iframe_test.html failed:
00:52:14.574 18825 worker/3  leak detected: ({"numberOfLiveDocuments":[1,3],"numberOfLiveFrames":[1,2],"numberOfLiveNodes":[4,27],"numberOfLivePausableObjects":[2,5],"numberOfLiveResourceFetchers":[1,3],"numberOfLiveResources":[0,3]})
00:52:14.574 18825 worker/3  asserts failed

This occurred for the first time here:

https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Linux%20Trusty%20Leak/16767
https://chromium.googlesource.com/chromium/src/+log/6a1e03470f144852659f994b7b6ed8b4b940f85c..126e07f1b5cb066842dd6cebb441c2bea1c136f8

I suspect the culprit in here:

https://chromium.googlesource.com/v8/v8/+log/9cf8abb7..3d6608ce


 
Cc: clemensh@chromium.org
Owner: ahaas@chromium.org
PTAL wasm folks
Cc: hpayer@chromium.org
As a temporary suppression we could add the three failing tests here:
https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/LeakExpectations?q=LeakExpectations&dr

+Hannes, do you more about how these webkit leak bots work? They seem to be no sanitizer leak checkers.

Comment 4 by ahaas@chromium.org, Mar 26 2018

The culprit seems to be https://crrev.com/c/958865. The CL is already hard to revert. I surpress the tests for now and start working on a fix.
Project Member

Comment 5 by bugdroid1@chromium.org, Mar 26 2018

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

commit bd0fff7ddac70721fd84af38c0a0f341c46bba36
Author: Andreas Haas <ahaas@chromium.org>
Date: Mon Mar 26 14:31:42 2018

Temporarily surpress memory leak check for wasm indexeddb test

Apparently some memory leak was introduced by https://crrev.com/c/958865.
Reverting this CL would be quite painful already. Therefore I surpress
the check for now to make the bots green again.

NOTRY=true
R=machenbach@chromium.org

Bug:  chromium:825741 
Change-Id: I7cb6ee3db03c234bcca6bade73aa0208c71e7b52
Reviewed-on: https://chromium-review.googlesource.com/980532
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545783}
[modify] https://crrev.com/bd0fff7ddac70721fd84af38c0a0f341c46bba36/third_party/WebKit/LayoutTests/LeakExpectations

Comment 6 by ahaas@chromium.org, Mar 27 2018

We found the leak. The WebAssembly CompilationState owns a DeferredHandleScope which contains a handle to the native context. The native context contains a reference to a compiled WebAssembly module, which owns the original CompilationState. This cycle cannot be collected by the GC because of the DeferredHandleScope. I will upload a fix tomorrow.
Project Member

Comment 7 by bugdroid1@chromium.org, Mar 28 2018

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

commit 071cecc048b6cf1034ce0cd0902b8d3b9c743ee9
Author: Kim-Anh Tran <kimanh@google.com>
Date: Wed Mar 28 12:10:45 2018

[wasm] Removing cyclic dependency caused by the CompilationState

Removes the deferred handle reference to the native context that
caused a cyclic dependency, which resulted in a memory leak. Instead of
keeping a reference to the native context, we use a phantom reference
to the WasmCompiledModule in order to get the context.
All foreground tasks are now registered in its own foreground task
manager, in order to make sure that we cancel all scheduled
foreground tasks as soon as the CompilationState is collected.

Bug:  chromium:825741 
Also-by: ahaas@chromium.org
Change-Id: Id69426a15280a14a1dc3ecd035415e7cfa61780b
Reviewed-on: https://chromium-review.googlesource.com/982622
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Commit-Queue: Kim-Anh Tran <kimanh@google.com>
Cr-Commit-Position: refs/heads/master@{#52270}
[modify] https://crrev.com/071cecc048b6cf1034ce0cd0902b8d3b9c743ee9/src/wasm/module-compiler.cc
[modify] https://crrev.com/071cecc048b6cf1034ce0cd0902b8d3b9c743ee9/src/wasm/module-compiler.h
[modify] https://crrev.com/071cecc048b6cf1034ce0cd0902b8d3b9c743ee9/src/wasm/wasm-code-manager.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 4 2018

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

commit 65e7fbb71a35882a2b36b20a1fb6505f70ff63ef
Author: Andreas Haas <ahaas@chromium.org>
Date: Wed Apr 04 13:23:53 2018

Re-enable memory leak check for wasm indexeddb test

The issue has been fixed in https://chromium-review.googlesource.com/982622.

R=machenbach@chromium.org

Bug:  chromium:825741 
Change-Id: I012a43e9b569600934ed29b269ee2f99805f6f12
Reviewed-on: https://chromium-review.googlesource.com/995277
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548032}
[modify] https://crrev.com/65e7fbb71a35882a2b36b20a1fb6505f70ff63ef/third_party/WebKit/LayoutTests/LeakExpectations

Comment 9 by ahaas@chromium.org, Apr 4 2018

Status: Fixed (was: Assigned)

Sign in to add a comment