Fix memory leak of Test objects.
am: ee59d49576
Change-Id: Ibad8c7f45dc51200a4efe847ca02539fa03ff7b1
diff --git a/Isolate.cpp b/Isolate.cpp
index 3eb63ac..e2e2f33 100644
--- a/Isolate.cpp
+++ b/Isolate.cpp
@@ -203,11 +203,11 @@
exit(ChildProcessFn(tests_[cur_test_index_]));
}
- size_t run_index = running_indices_.top();
- running_indices_.pop();
+ size_t run_index = running_indices_.back();
+ running_indices_.pop_back();
Test* test = new Test(tests_[cur_test_index_], cur_test_index_, run_index, read_fd.release());
+ running_by_pid_.emplace(pid, test);
running_[run_index] = test;
- running_by_pid_[pid] = test;
running_by_test_index_[cur_test_index_] = test;
pollfd* pollfd = &running_pollfds_[run_index];
@@ -247,7 +247,8 @@
LOG(FATAL) << "Pid " << pid << " was not spawned by the isolation framework.";
}
- Test* test = entry->second;
+ std::unique_ptr<Test>& test_ptr = entry->second;
+ Test* test = test_ptr.get();
test->Stop();
// Read any leftover data.
@@ -312,8 +313,8 @@
}
finished_tests++;
size_t test_index = test->test_index();
- finished_.emplace(test_index, test);
- running_indices_.push(test->run_index());
+ finished_.emplace(test_index, test_ptr.release());
+ running_indices_.push_back(test->run_index());
// Remove it from all of the running indices.
size_t run_index = test->run_index();
@@ -337,7 +338,7 @@
void Isolate::CheckTestsTimeout() {
for (auto& entry : running_by_pid_) {
- Test* test = entry.second;
+ Test* test = entry.second.get();
if (test->result() == TEST_TIMEOUT) {
continue;
}
@@ -388,8 +389,9 @@
running_.resize(job_count);
running_pollfds_.resize(job_count);
memset(running_pollfds_.data(), 0, running_pollfds_.size() * sizeof(pollfd));
+ running_indices_.clear();
for (size_t i = 0; i < job_count; i++) {
- running_indices_.push(i);
+ running_indices_.push_back(i);
}
finished_.clear();
@@ -417,7 +419,7 @@
ColoredPrintf(color, prefix);
printf(" %s, listed below:\n", PluralizeString(total, " test").c_str());
for (const auto& entry : finished_) {
- const Test* test = entry.second;
+ const Test* test = entry.second.get();
if (match_func(*test)) {
ColoredPrintf(color, prefix);
printf(" %s", test->name().c_str());
@@ -585,7 +587,7 @@
std::vector<CaseInfo> cases;
CaseInfo* info = nullptr;
for (const auto& entry : finished_) {
- const Test* test = entry.second;
+ const Test* test = entry.second.get();
const std::string& case_name = test->case_name();
if (test->result() == TEST_XFAIL) {
// Skip XFAIL tests.
diff --git a/Isolate.h b/Isolate.h
index 7025afb..a0d7891 100644
--- a/Isolate.h
+++ b/Isolate.h
@@ -21,6 +21,7 @@
#include <sys/types.h>
#include <map>
+#include <memory>
#include <stack>
#include <string>
#include <tuple>
@@ -91,11 +92,11 @@
std::vector<Test*> running_;
std::vector<pollfd> running_pollfds_;
- std::stack<size_t> running_indices_;
- std::unordered_map<pid_t, Test*> running_by_pid_;
+ std::vector<size_t> running_indices_;
+ std::unordered_map<pid_t, std::unique_ptr<Test>> running_by_pid_;
std::map<size_t, Test*> running_by_test_index_;
- std::map<size_t, Test*> finished_;
+ std::map<size_t, std::unique_ptr<Test>> finished_;
static constexpr useconds_t MIN_USECONDS_WAIT = 1000;
};
diff --git a/tests/SystemTests.cpp b/tests/SystemTests.cpp
index 61b0f13..194731c 100644
--- a/tests/SystemTests.cpp
+++ b/tests/SystemTests.cpp
@@ -15,6 +15,7 @@
*/
#include <fcntl.h>
+#include <malloc.h>
#include <signal.h>
#include <stdio.h>
#include <sys/types.h>
@@ -27,6 +28,7 @@
#include <vector>
#include <android-base/file.h>
+#include <android-base/strings.h>
#include <android-base/test_utils.h>
#include <gtest/gtest.h>
@@ -978,6 +980,52 @@
ASSERT_EQ(0, WEXITSTATUS(status));
}
+TEST_F(SystemTests, verify_memory) {
+ // This test verifies that memory isn't leaking when running repeatedly.
+ std::vector<const char*> args{"--gtest_filter=*.DISABLED_memory",
+ "--gtest_also_run_disabled_tests", "--gtest_repeat=400"};
+ ExecAndCapture(args);
+ ASSERT_EQ(0, exitcode_) << "Test output:\n" << raw_output_;
+ std::vector<std::string> lines(android::base::Split(raw_output_, "\n"));
+
+ constexpr static size_t kMaxLeakBytes = 32 * 1024;
+ size_t memory_iteration = 0;
+ size_t memory_start = 0;
+ size_t memory_last = 0;
+ for (auto& line : lines) {
+ size_t memory;
+ if (android::base::StartsWith(line, "Allocated ") &&
+ sscanf(line.c_str(), "Allocated %zu", &memory) == 1) {
+ if (memory_iteration == 0) {
+ memory_start = memory;
+ } else {
+ // Check the increase from the last loop.
+ if (memory > memory_last) {
+ ASSERT_GT(kMaxLeakBytes, memory - memory_last)
+ << "On iteration " << memory_iteration << " memory increased beyond expected value."
+ << std::endl
+ << "Last memory bytes " << memory_last << std::endl
+ << "Current memory bytes " << memory;
+ }
+ // Check the increase from the first loop.
+ if (memory > memory_start) {
+ ASSERT_GT(kMaxLeakBytes, memory - memory_start)
+ << "On iteration " << memory_iteration
+ << " total memory increased beyond expected value." << std::endl
+ << "Starting memory bytes " << memory_start << std::endl
+ << "Current memory bytes " << memory;
+ }
+ }
+ memory_last = memory;
+ memory_iteration++;
+ }
+ }
+ ASSERT_EQ(400, memory_iteration)
+ << "Did not find the expected 400 lines of memory data." << std::endl
+ << "Raw output:" << std::endl
+ << raw_output_;
+}
+
// These tests are used by the verify_disabled tests.
TEST_F(SystemTests, always_pass) {}
@@ -1156,5 +1204,14 @@
ASSERT_EQ(1, 0);
}
+TEST(SystemTestsMemory, DISABLED_memory) {
+ struct mallinfo info = mallinfo();
+#if defined(__ANDROID__)
+ printf("Allocated %zu\n", info.uordblks);
+#else
+ printf("Allocated %d\n", info.uordblks);
+#endif
+}
+
} // namespace gtest_extras
} // namespace android
