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

Issue 662388 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug


Sign in to add a comment

Fix suppressed memory leaks

Project Member Reported by machenb...@chromium.org, Nov 4 2016

Issue description

Example:
https://build.chromium.org/p/tryserver.v8/builders/v8_linux64_sanitizer_coverage_rel/builds/10944

Output example:
Test: cctest/test-run-wasm-module/Run_WasmModule_CallMain_recursive
Flags: --stress-opt --always-opt
Command: /b/swarming/w/iriMP7jH/out/Release/cctest --random-seed=254731550 --stress-opt --always-opt test-run-wasm-module/Run_WasmModule_CallMain_recursive --nohard-abort --nodead-code-elimination --nofold-constants --invoke-weak-callbacks --omit-quit

Build environment:
 GYP_DEFINES: component=static_library use_goma=1 target_arch=x64 clang=1 asan=1 sanitizer_coverage=bb coverage=1 lsan=1 fastbuild=1 test_isolation_mode=prepare

Run #1
Exit code: 1
Result: FAIL
Expected outcomes: PASS
Duration: 00:00:519

Stderr:

=================================================================
==3008==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 264 byte(s) in 1 object(s) allocated from:
    #0 0x559f6b in operator new(unsigned long) (/b/swarming/w/iriMP7jH/out/Release/cctest+0x559f6b)
    #1 0x234acb8 in v8::internal::wasm::DecodeWasmModule(v8::internal::Isolate*, unsigned char const*, unsigned char const*, bool, v8::internal::wasm::ModuleOrigin) /b/build/slave/sancov_linux64/build/v8/out/Release/../../src/wasm/module-decoder.cc:1080:24
    #2 0x239341c in v8::internal::wasm::CreateModuleObjectFromBytes(v8::internal::Isolate*, unsigned char const*, unsigned char const*, v8::internal::wasm::ErrorThrower*, v8::internal::wasm::ModuleOrigin, v8::internal::Handle<v8::internal::Script>, unsigned char const*, unsigned char const*) /b/build/slave/sancov_linux64/build/v8/out/Release/../../src/wasm/wasm-module.cc:2025:25

 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 4 2016

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

commit 3019b49764c652ed2e894def25f480ad724cd68a
Author: vogelheim <vogelheim@chromium.org>
Date: Fri Nov 04 18:53:40 2016

Fix memory leak in test-scanner.cc.

BUG= chromium:662388 

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

[modify] https://crrev.com/3019b49764c652ed2e894def25f480ad724cd68a/test/cctest/parsing/test-scanner.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Nov 8 2016

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

commit 0ab9ecc23ada2a8614afe46b86bcdd8cfc9cbbb4
Author: ahaas <ahaas@chromium.org>
Date: Tue Nov 08 10:33:42 2016

[wasm] Fix a memory leak in test-run-wasm-module.

The memory leak is fixed by calling the GC at the end of the tests. The GC collects the WasmModuleWrapper objects, which deallocates WasmModule c++ object. For the mjsunit tests the GC is already called because of the --invoke_weak_callbacks flag.

BUG= chromium:662388 

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

[modify] https://crrev.com/0ab9ecc23ada2a8614afe46b86bcdd8cfc9cbbb4/src/wasm/wasm-module.cc
[modify] https://crrev.com/0ab9ecc23ada2a8614afe46b86bcdd8cfc9cbbb4/test/cctest/wasm/test-run-wasm-module.cc

Comment 3 by ahaas@chromium.org, Nov 8 2016

Status: Fixed (was: Assigned)
Owner: vogelheim@chromium.org
Status: Assigned (was: Fixed)
I still see some leaks, e.g.:
https://build.chromium.org/p/tryserver.v8/builders/v8_linux64_sanitizer_coverage_rel/builds/10980/steps/Check/logs/Bookmarks
Project Member

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

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

commit 56daccb8368c459087f20b1177ee22bea4de118e
Author: vogelheim <vogelheim@chromium.org>
Date: Thu Nov 24 14:28:53 2016

Fix memory leak in cctest/parsing/test-scanner.

BUG= chromium:662388 

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

[modify] https://crrev.com/56daccb8368c459087f20b1177ee22bea4de118e/test/cctest/parsing/test-scanner.cc

Owner: machenb...@chromium.org
@machenbach: The leaks in test-scanner have disappeared w/ #5 from the v8_linux64_sanitizer_coverage_rel bot; but meanwhile others have shown up. Not sure what to do with the bug, so assignment back to you.
Blockedon: v8:5754
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 19 2016

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

commit 2a19ad3603f355c0069f15840ec3d7b2c53fe414
Author: yangguo <yangguo@chromium.org>
Date: Mon Dec 19 11:58:28 2016

Fix memory leak in logging-unittest.

R=cbruni@chromium.org
BUG= chromium:662388 

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

[modify] https://crrev.com/2a19ad3603f355c0069f15840ec3d7b2c53fe414/test/unittests/base/logging-unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 19 2016

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

commit 8ac9e55aa63448cc3dd58060d8b084817db9d0b6
Author: yangguo <yangguo@chromium.org>
Date: Mon Dec 19 12:04:19 2016

[serializer] fix leak in test.

BUG= chromium:662388 
R=cbruni@chromium.org

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

[modify] https://crrev.com/8ac9e55aa63448cc3dd58060d8b084817db9d0b6/test/cctest/test-serialize.cc

Cc: fmea...@chromium.org cbruni@chromium.org
Labels: -Pri-2 Pri-1
Owner: alph@chromium.org
Some leaks seem to have been introduced by
https://chromium.googlesource.com/v8/v8/+/50e50db7fdf801460f83e9ec2f70d87e25de6827

See https://codereview.chromium.org/2592663004/

Example:
https://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_triggered/builds/13258/steps/Check/logs/DeleteAllCpuProfiles

Could you please provide a fix? Also CC'ing reviewers of that CL.
Cc: alph@chromium.org
Owner: yangguo@chromium.org
Yang, could you please find an owner for this?
Cc: l...@chromium.org

Comment 13 by l...@chromium.org, Jan 25 2017

Is the leak DeleteAllCpuProfiles still exists, the example link above is 404 and I can't find any on https://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng_triggered

I remembered Yang fixed memory leak in this CL https://codereview.chromium.org/2586923002/ but got reverted.
Cc: bradnelson@chromium.org titzer@chromium.org
Owner: titzer@chromium.org
Maybe they are fixed now. We can basically look at recent runs of:
https://build.chromium.org/p/tryserver.v8/builders/v8_linux64_sanitizer_coverage_rel/

There are a few wasm leaks now.

Comment 16 by alph@chromium.org, Jan 25 2017

As for leaks in ProfileGenerator
There's a bug on that issue https://bugs.chromium.org/p/v8/issues/detail?id=5753
I've made a fix for it https://codereview.chromium.org/2655963003/
Blockedon: 686338
We should find a way to suppress the known leaks and get lsan turned back on. Otherwise we seem to have a race of fixes and new breakages so that we never get to a green point.
Leak detection is now back on with some suppressions. Please remove the suppressions one by one:
https://chromium.googlesource.com/v8/v8/+/e196d00df587b6513eb79059cd96e259dd0a763f/tools/memory/lsan/suppressions.txt

A few of the things seem wasm related, so lets get rid of those first.
ok - reverting :( - please wait for the reland 
Project Member

Comment 22 by bugdroid1@chromium.org, Jan 31 2017

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

commit 9cf7cb4c6784372e5820049cf91423c9b58d15e1
Author: machenbach <machenbach@chromium.org>
Date: Tue Jan 31 15:12:08 2017

Revert of [test] Add back lsan leak detection (patchset #4 id:60001 of https://codereview.chromium.org/2592663004/ )

Reason for revert:
Breaks mac asan. Need to keep leak check off on mac.

Original issue's description:
> [test] Add back lsan leak detection
>
> BUG= chromium:662388 
> TBR=yangguo@chromium.org, glider@chromium.org, titzer@chromium.org
>
> Review-Url: https://codereview.chromium.org/2592663004
> Cr-Commit-Position: refs/heads/master@{#42815}
> Committed: https://chromium.googlesource.com/v8/v8/+/e196d00df587b6513eb79059cd96e259dd0a763f

TBR=yangguo@chromium.org,glider@chromium.org,titzer@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= chromium:662388 

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

[modify] https://crrev.com/9cf7cb4c6784372e5820049cf91423c9b58d15e1/gni/isolate.gni
[modify] https://crrev.com/9cf7cb4c6784372e5820049cf91423c9b58d15e1/gypfiles/isolate.gypi
[delete] https://crrev.com/e196d00df587b6513eb79059cd96e259dd0a763f/tools/memory/lsan/suppressions.txt
[modify] https://crrev.com/9cf7cb4c6784372e5820049cf91423c9b58d15e1/tools/run-tests.py
[modify] https://crrev.com/9cf7cb4c6784372e5820049cf91423c9b58d15e1/tools/testrunner/testrunner.isolate

Summary: Fix suppressed memory leaks (was: Fix memory leaks found by v8_linux64_sanitizer_coverage_rel)
Owner: yangguo@chromium.org
Please help find a new owner for this. See current suppressions in:
https://chromium.googlesource.com/v8/v8/+/e196d00df587b6513eb79059cd96e259dd0a763f/tools/memory/lsan/suppressions.txt
Blockedon: v8:6314
Blockedon: v8:6315
Blockedon: v8:6316
Blockedon: v8:6317
Cc: kozyatinskiy@chromium.org dgozman@chromium.org
Bisected locally, and all but one leak were fixed by:
https://codereview.chromium.org/2819423005

Does that sound right?

The remaining leak in unittests/ValueSerializerTest.DecodeArrayBufferOOM is also fixed in HEAD, so I'll just remove all suppressions now.
Owner: machenb...@chromium.org
Assigning back to myself to clean up suppressions.
Status: Verified (was: Assigned)
Yes, my CL potentially could fix some memory leaks. Thanks a lot!

Sign in to add a comment