Fix memory leak on HttpFetcher and ProxyResolver.
The current HttpFetcher and ProxyResolver code uses
google::protobuf::Closure callbacks created with NewCallback. These
callbacks will self-delete them when you call Run(), leaking the
callback if that doesn't happens.
This patch replaces all the NewCallback() calls by
NewPermanentCallback(), which won't delete the callback after running
it. It then adds a new utils::GlibDestroyClosure() function to use in
conjunction with the existing utils::GlibRunClosure() to schedule
callbacks from the glib main loop without leaking them.
Finally, this patch fixes a use-after-free on the
AbortingHttpFetcherTestDelegate class only affecting unit tests.
Other minor linting errors fixed.
BUG=chromium:378548
TEST=`FEATURES="test" USE="clang asan" emerge-link update_engine` doesn't complain about HttpFetcher.
Change-Id: Ica3265aca42f07811b7dff6131f9a43ab06269aa
Reviewed-on: https://chromium-review.googlesource.com/202062
Tested-by: Alex Deymo <deymo@chromium.org>
Reviewed-by: Gilad Arnold <garnold@chromium.org>
Commit-Queue: Alex Deymo <deymo@chromium.org>
diff --git a/http_fetcher_unittest.cc b/http_fetcher_unittest.cc
index 74c6a16..d7713c5 100644
--- a/http_fetcher_unittest.cc
+++ b/http_fetcher_unittest.cc
@@ -35,10 +35,10 @@
using std::vector;
using base::TimeDelta;
-using testing::_;
-using testing::SetArgumentPointee;
using testing::DoAll;
using testing::Return;
+using testing::SetArgumentPointee;
+using testing::_;
namespace {
@@ -198,6 +198,7 @@
: mock_connection_manager_(&fake_system_state_) {
fake_system_state_.set_connection_manager(&mock_connection_manager_);
}
+ virtual ~AnyHttpFetcherTest() {}
virtual HttpFetcher* NewLargeFetcher(size_t num_proxies) = 0;
HttpFetcher* NewLargeFetcher() {
@@ -611,8 +612,9 @@
EXPECT_FALSE(once_);
EXPECT_TRUE(callback_once_);
callback_once_ = false;
- // |fetcher| can be destroyed during this callback.
- fetcher_.reset(NULL);
+ // The fetcher could have a callback scheduled on the ProxyResolver that
+ // can fire after this callback. We wait until the end of the test to
+ // delete the fetcher.
}
void TerminateTransfer() {
CHECK(once_);