librados: release pimpl pointer in destructor

before this change, the librados applications are responsible to call
`AioCompletion::release()` explicitly to release its internal pimpl
pointer. this is error prone and not intuitive.

after this change, the destructor of `AioCompletion` and
`PoolAsyncCompletion` will do this automatically. while
`AioCompletion::release()` and `PoolAsyncCompletion::release()` still
delete the instance as they did before. so this change is backward
compatible, as existing librados clients can still use `ptr->release()`
to free the completion instance, while new clients can just `delete
ptr`.

librados_test_stub is updated accordingly to match the new model

Signed-off-by: Kefu Chai <kchai@redhat.com>

diff --git a/src/include/rados/librados.hpp b/src/include/rados/librados.hpp
index 69e9c249c67e30953f862afa110919c2f912d63a..2cda4d84756e99d250d875668d24ca234c8e10a1 100644 (file)
--- a/src/include/rados/librados.hpp
+++ b/src/include/rados/librados.hpp
@@ -191,6 +191,7 @@ inline namespace v14_2_0 {
 
   struct CEPH_RADOS_API AioCompletion {
     AioCompletion(AioCompletionImpl *pc_) : pc(pc_) {}
+    ~AioCompletion();
     int set_complete_callback(void *cb_arg, callback_t cb);
     int set_safe_callback(void *cb_arg, callback_t cb)
       __attribute__ ((deprecated));
@@ -211,6 +212,7 @@ inline namespace v14_2_0 {
 
   struct CEPH_RADOS_API PoolAsyncCompletion {
     PoolAsyncCompletion(PoolAsyncCompletionImpl *pc_) : pc(pc_) {}
+    ~PoolAsyncCompletion();
     int set_callback(void *cb_arg, callback_t cb);
     int wait();
     bool is_complete();

diff --git a/src/librados/librados_c.cc b/src/librados/librados_c.cc
index bbdd91209556f1040f54494d1d013e44defdef5b..f950b8f247630908d53ad71db2d2df58bd76f04e 100644 (file)
--- a/src/librados/librados_c.cc
+++ b/src/librados/librados_c.cc
@@ -3024,6 +3024,7 @@ extern "C" int _rados_aio_unlock(rados_ioctx_t io, const char *o, const char *na
   librados::IoCtx ctx;
   librados::IoCtx::from_rados_ioctx_t(io, ctx);
   librados::AioCompletionImpl *comp = (librados::AioCompletionImpl*)completion;
+  comp->get();
   librados::AioCompletion c(comp);
   int retval = ctx.aio_unlock(o, name, cookie, &c);
   tracepoint(librados, rados_aio_unlock_exit, retval);

diff --git a/src/librados/librados_cxx.cc b/src/librados/librados_cxx.cc
index 4e240fe0c027c7435ef2b2e46f1f12508be7c203..0c780a2bd719704e84f5c79a0461a897e7d61804 100644 (file)
--- a/src/librados/librados_cxx.cc
+++ b/src/librados/librados_cxx.cc
@@ -962,6 +962,12 @@ uint32_t librados::NObjectIterator::get_pg_hash_position() const
 const librados::NObjectIterator librados::NObjectIterator::__EndObjectIterator(NULL);
 
 ///////////////////////////// PoolAsyncCompletion //////////////////////////////
+librados::PoolAsyncCompletion::PoolAsyncCompletion::~PoolAsyncCompletion()
+{
+  auto c = reinterpret_cast<PoolAsyncCompletionImpl *>(pc);
+  c->release();
+}
+
 int librados::PoolAsyncCompletion::PoolAsyncCompletion::set_callback(void *cb_arg,
                                                                     rados_callback_t cb)
 {
@@ -989,12 +995,16 @@ int librados::PoolAsyncCompletion::PoolAsyncCompletion::get_return_value()
 
 void librados::PoolAsyncCompletion::PoolAsyncCompletion::release()
 {
-  PoolAsyncCompletionImpl *c = (PoolAsyncCompletionImpl *)pc;
-  c->release();
   delete this;
 }
 
 ///////////////////////////// AioCompletion //////////////////////////////
+librados::AioCompletion::AioCompletion::~AioCompletion()
+{
+  auto c = reinterpret_cast<AioCompletionImpl *>(pc);
+  c->release();
+}
+
 int librados::AioCompletion::AioCompletion::set_complete_callback(void *cb_arg, rados_callback_t cb)
 {
   AioCompletionImpl *c = (AioCompletionImpl *)pc;
@@ -1075,8 +1085,6 @@ uint64_t librados::AioCompletion::AioCompletion::get_version64()
 
 void librados::AioCompletion::AioCompletion::release()
 {
-  AioCompletionImpl *c = (AioCompletionImpl *)pc;
-  c->release();
   delete this;
 }
 

diff --git a/src/test/librados_test_stub/LibradosTestStub.cc b/src/test/librados_test_stub/LibradosTestStub.cc
index be7af75e4a250e0cb887d0eea6628fac6cc5e119..c80dd6fe52b107c3006ed23230b7d83682824894 100644 (file)
--- a/src/test/librados_test_stub/LibradosTestStub.cc
+++ b/src/test/librados_test_stub/LibradosTestStub.cc
@@ -345,9 +345,13 @@ extern "C" int rados_wait_for_latest_osdmap(rados_t cluster) {
 
 namespace librados {
 
-void AioCompletion::release() {
-  AioCompletionImpl *c = reinterpret_cast<AioCompletionImpl *>(pc);
+AioCompletion::~AioCompletion()
+{
+  auto c = reinterpret_cast<AioCompletionImpl *>(pc);
   c->release();
+}
+
+void AioCompletion::release() {
   delete this;
 }