]> git.apps.os.sepia.ceph.com Git - ceph-ci.git/commitdiff
librbd: correct memory leaks discovered via valgrind
authorJason Dillaman <dillaman@redhat.com>
Wed, 17 Feb 2016 15:35:47 +0000 (10:35 -0500)
committerJason Dillaman <dillaman@redhat.com>
Thu, 18 Feb 2016 22:46:35 +0000 (17:46 -0500)
Signed-off-by: Jason Dillaman <dillaman@redhat.com>
src/librbd/journal/Replay.cc
src/librbd/journal/Replay.h
src/test/librbd/test_librbd.cc
src/test/librbd/test_mock_ExclusiveLock.cc
src/test/librbd/test_mock_fixture.cc

index bd6e081d4df2ba2345223cce48774c07baac0ab3..96186ca0c2daa75d2d56a039ae60ae375ed6f208 100644 (file)
@@ -542,7 +542,10 @@ Context *Replay<I>::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 <typename I>
@@ -573,6 +576,9 @@ void Replay<I>::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);
index c8c58cbe2d2a114d08355a9bd215f36542ecfb3a..f4ac51af622ed7ec6fa927520449ef1b38821492 100644 (file)
@@ -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<uint64_t> OpTids;
index 4fcf6773d79a411fe86c6e181cb7b49acf50ea59..25c28f8d266156d755a8d1ec3c1b4b48fe3cf044 100644 (file)
@@ -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;
index f57dca987b21fe82305653ce9a43a651a99f2c24..0bd61fe653079bf9b85a3d07a7b2b7bb0ea785ca 100644 (file)
@@ -17,8 +17,8 @@ namespace exclusive_lock {
 template<typename T>
 struct BaseRequest {
   static std::list<T *> 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<MockImageCtx> : public BaseRequest<ReleaseRequest<MockImag
 #include "librbd/ExclusiveLock.cc"
 template class librbd::ExclusiveLock<librbd::MockImageCtx>;
 
+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());
 
index f5a2394fc32c2fe126903cdb7dcd54dd19208427..cd03ac6f3a8aa4dcaef6ac1fa59b6e84469ba945 100644 (file)
@@ -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() {