From 922072aeb568abc8dd717ff2d4a2eefc941a7121 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Wed, 17 Feb 2016 10:35:47 -0500 Subject: [PATCH] librbd: correct memory leaks discovered via valgrind Signed-off-by: Jason Dillaman --- src/librbd/journal/Replay.cc | 8 +++++++- src/librbd/journal/Replay.h | 1 + src/test/librbd/test_librbd.cc | 1 + src/test/librbd/test_mock_ExclusiveLock.cc | 19 +++++++++++++++---- src/test/librbd/test_mock_fixture.cc | 2 ++ 5 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/librbd/journal/Replay.cc b/src/librbd/journal/Replay.cc index bd6e081d4df..96186ca0c2d 100644 --- a/src/librbd/journal/Replay.cc +++ b/src/librbd/journal/Replay.cc @@ -542,7 +542,10 @@ Context *Replay::create_op_context_callback(uint64_t op_tid, *op_event = &m_op_events[op_tid]; (*op_event)->on_start_safe = on_safe; - return new C_OpOnComplete(this, op_tid); + + Context *on_op_complete = new C_OpOnComplete(this, op_tid); + (*op_event)->on_op_complete = on_op_complete; + return on_op_complete; } template @@ -573,6 +576,9 @@ void Replay::handle_op_complete(uint64_t op_tid, int r) { // skipped upon error -- so clean up if non-null delete op_event.on_op_finish_event; + if (r == -ERESTART) { + delete op_event.on_op_complete; + } if (op_event.on_finish_ready != nullptr) { op_event.on_finish_ready->complete(0); diff --git a/src/librbd/journal/Replay.h b/src/librbd/journal/Replay.h index c8c58cbe2d2..f4ac51af622 100644 --- a/src/librbd/journal/Replay.h +++ b/src/librbd/journal/Replay.h @@ -46,6 +46,7 @@ private: Context *on_start_safe = nullptr; Context *on_finish_ready = nullptr; Context *on_finish_safe = nullptr; + Context *on_op_complete = nullptr; }; typedef std::list OpTids; diff --git a/src/test/librbd/test_librbd.cc b/src/test/librbd/test_librbd.cc index 4fcf6773d79..25c28f8d266 100644 --- a/src/test/librbd/test_librbd.cc +++ b/src/test/librbd/test_librbd.cc @@ -3761,6 +3761,7 @@ TEST_F(TestLibRBD, ExclusiveLockTransition) comps.pop_front(); ASSERT_EQ(0, comp->wait_for_complete()); ASSERT_EQ(1, comp->is_complete()); + comp->release(); } librbd::Image image3; diff --git a/src/test/librbd/test_mock_ExclusiveLock.cc b/src/test/librbd/test_mock_ExclusiveLock.cc index f57dca987b2..0bd61fe6530 100644 --- a/src/test/librbd/test_mock_ExclusiveLock.cc +++ b/src/test/librbd/test_mock_ExclusiveLock.cc @@ -17,8 +17,8 @@ namespace exclusive_lock { template struct BaseRequest { static std::list s_requests; - Context *on_lock_unlock; - Context *on_finish; + Context *on_lock_unlock = nullptr; + Context *on_finish = nullptr; static T* create(MockImageCtx &image_ctx, const std::string &cookie, Context *on_lock_unlock, Context *on_finish) { @@ -55,9 +55,16 @@ struct ReleaseRequest : public BaseRequest; +ACTION_P(FinishLockUnlock, request) { + if (request->on_lock_unlock != nullptr) { + request->on_lock_unlock->complete(0); + } +} + namespace librbd { using ::testing::_; +using ::testing::DoAll; using ::testing::Invoke; using ::testing::InSequence; using ::testing::Return; @@ -86,7 +93,8 @@ public: MockAcquireRequest &acquire_request, int r) { expect_get_watch_handle(mock_image_ctx); EXPECT_CALL(acquire_request, send()) - .WillOnce(FinishRequest(&acquire_request, r, &mock_image_ctx)); + .WillOnce(DoAll(FinishLockUnlock(&acquire_request), + FinishRequest(&acquire_request, r, &mock_image_ctx))); if (r == 0) { expect_notify_acquired_lock(mock_image_ctx); expect_unblock_writes(mock_image_ctx); @@ -97,7 +105,8 @@ public: MockReleaseRequest &release_request, int r, bool shutting_down = false) { EXPECT_CALL(release_request, send()) - .WillOnce(FinishRequest(&release_request, r, &mock_image_ctx)); + .WillOnce(DoAll(FinishLockUnlock(&release_request), + FinishRequest(&release_request, r, &mock_image_ctx))); if (r == 0) { if (shutting_down) { expect_unblock_writes(mock_image_ctx); @@ -538,6 +547,7 @@ TEST_F(TestMockExclusiveLock, ConcurrentRequests) { // fail the try_lock ASSERT_EQ(0, wait_for_send_ctx1.wait()); + try_lock_acquire.on_lock_unlock->complete(0); try_lock_acquire.on_finish->complete(-EINVAL); ASSERT_EQ(-EINVAL, try_request_ctx1.wait()); @@ -548,6 +558,7 @@ TEST_F(TestMockExclusiveLock, ConcurrentRequests) { // proceed with the release ASSERT_EQ(0, wait_for_send_ctx2.wait()); + release.on_lock_unlock->complete(0); release.on_finish->complete(0); ASSERT_EQ(0, release_lock_ctx1.wait()); diff --git a/src/test/librbd/test_mock_fixture.cc b/src/test/librbd/test_mock_fixture.cc index f5a2394fc32..cd03ac6f3a8 100644 --- a/src/test/librbd/test_mock_fixture.cc +++ b/src/test/librbd/test_mock_fixture.cc @@ -37,6 +37,8 @@ void TestMockFixture::SetUpTestCase() { void TestMockFixture::TearDownTestCase() { TestFixture::TearDownTestCase(); librados_test_stub::set_rados_client(s_test_rados_client); + s_test_rados_client->put(); + s_test_rados_client.reset(); } void TestMockFixture::SetUp() { -- 2.39.5