Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(553)

Issue 53089: Fixed test memory leaks (Closed)

Created:
11 years, 9 months ago by Christian Plesner Hansen
Modified:
9 years, 7 months ago
CC:
v8-dev
Visibility:
Public.

Description

Fixed a bunch of memory leaks in tests, including: - String traversal test data (now in a zone) - Debug message thread (now joined on exit) - Threading test threads (now joined on exit) - Changed message tests framework to cope with valgrind Also, fixed a bug where we'd try to delete stack-allocated objects when tearing down v8. Good times.

Patch Set 1 #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+104 lines, -31 lines) Patch
M include/v8.h View 2 chunks +12 lines, -2 lines 2 comments Download
M samples/shell.cc View 2 chunks +8 lines, -2 lines 0 comments Download
M src/api.cc View 1 chunk +6 lines, -0 lines 0 comments Download
M src/debug.h View 4 chunks +4 lines, -1 line 0 comments Download
M src/debug.cc View 3 chunks +17 lines, -4 lines 2 comments Download
M src/flag-definitions.h View 1 chunk +1 line, -1 line 0 comments Download
M src/flags.cc View 7 chunks +18 lines, -13 lines 2 comments Download
M src/log.h View 2 chunks +4 lines, -1 line 0 comments Download
M src/log.cc View 3 chunks +2 lines, -3 lines 0 comments Download
M src/platform-linux.cc View 1 chunk +1 line, -0 lines 0 comments Download
M src/v8.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M src/zone.h View 1 chunk +3 lines, -0 lines 0 comments Download
M src/zone-inl.h View 1 chunk +6 lines, -0 lines 0 comments Download
M test/cctest/cctest.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M test/cctest/test-api.cc View 2 chunks +5 lines, -1 line 0 comments Download
M test/cctest/test-strings.cc View 6 chunks +7 lines, -2 lines 0 comments Download
M test/message/testcfg.py View 2 chunks +6 lines, -1 line 2 comments Download

Messages

Total messages: 3 (0 generated)
Christian Plesner Hansen
11 years, 9 months ago (2009-03-26 06:51:12 UTC) #1
Søren Thygesen Gjesse
LGTM Thanks for getting all this right. http://codereview.chromium.org/53089/diff/1/2 File include/v8.h (right): http://codereview.chromium.org/53089/diff/1/2#newcode2042 Line 2042: * ...
11 years, 9 months ago (2009-03-26 10:50:09 UTC) #2
Christian Plesner Hansen
11 years, 9 months ago (2009-03-27 00:24:26 UTC) #3
http://codereview.chromium.org/53089/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/53089/diff/1/2#newcode2042
Line 2042: * a process.
Done.  I left it out originally because I had no idea what to write but I've put
something general.

http://codereview.chromium.org/53089/diff/1/5
File src/debug.cc (right):

http://codereview.chromium.org/53089/diff/1/5#newcode1742
Line 1742: message_thread_->Stop();
Done.

Technically it's not a leak as long as there's a static variable pointing to it.
 But yes, it might as well be deleted.

http://codereview.chromium.org/53089/diff/1/8
File src/flags.cc (right):

http://codereview.chromium.org/53089/diff/1/8#newcode61
Line 61: bool owns_ptr_;
Done; it wasn't obvious.

http://codereview.chromium.org/53089/diff/1/18
File test/message/testcfg.py (right):

http://codereview.chromium.org/53089/diff/1/18#newcode43
Line 43: 
Done

Powered by Google App Engine
This is Rietveld 408576698