rgw: destroy get_obj handle in copy_obj()wip-6176

Fixes: #6176
Backport: dumpling
We take different code paths in copy_obj, make sure we close the handle
when we exit the function. Move the call to finish_get_obj() out of
copy_obj_data() as we don't create the handle there, so that should
makes code less confusing and less prone to errors.
Also, note that RGWRados::get_obj() also calls finish_get_obj(). For
everything to work in concert we need to pass a pointer to the handle
and not the handle itself. Therefore we needed to also change the call
to copy_obj_data().

Signed-off-by: Yehuda Sadeh <yehuda@inktank.com>

2 files changed, 23 insertions, 7 deletions

diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc
index 03cc1ebfdb3..c6997f7b039 100644
--- a/src/rgw/rgw_rados.cc
+++ b/src/rgw/rgw_rados.cc
@@ -2479,6 +2479,22 @@ static void set_copy_attrs(map<string, bufferlist>& src_attrs, map<string, buffe
   }
 }
 
+class GetObjHandleDestructor {
+  RGWRados *store;
+  void **handle;
+
+public:
+    GetObjHandleDestructor(RGWRados *_store) : store(_store), handle(NULL) {}
+    ~GetObjHandleDestructor() {
+      if (handle) {
+        store->finish_get_obj(handle);
+      }
+    }
+    void set_handle(void **_h) {
+      handle = _h;
+    }
+};
+
 /**
  * Copy an object.
  * dest_obj: the object to copy into
@@ -2533,6 +2549,7 @@ int RGWRados::copy_obj(void *ctx,
   ldout(cct, 5) << "Copy object " << src_obj.bucket << ":" << src_obj.object << " => " << dest_obj.bucket << ":" << dest_obj.object << dendl;
 
   void *handle = NULL;
+  GetObjHandleDestructor handle_destructor(this);
 
   map<string, bufferlist> src_attrs;
   off_t ofs = 0;
@@ -2542,6 +2559,8 @@ int RGWRados::copy_obj(void *ctx,
                   mod_ptr, unmod_ptr, &lastmod, if_match, if_nomatch, &total_len, &obj_size, NULL, &handle, err);
     if (ret < 0)
       return ret;
+
+    handle_destructor.set_handle(&handle);
   } else {
     /* source is in a different region, copy it there */
 
@@ -2684,7 +2703,7 @@ set_err_state:
 
     return 0;
   } else if (copy_data) { /* refcounting tail wouldn't work here, just copy the data */
-    return copy_obj_data(ctx, handle, end, dest_obj, src_obj, mtime, src_attrs, category, ptag, err);
+    return copy_obj_data(ctx, &handle, end, dest_obj, src_obj, mtime, src_attrs, category, ptag, err);
   }
 
   map<uint64_t, RGWObjManifestPart>::iterator miter = astate->manifest.objs.begin();
@@ -2789,7 +2808,7 @@ done_ret:
 
 
 int RGWRados::copy_obj_data(void *ctx,
-	       void *handle, off_t end,
+	       void **handle, off_t end,
                rgw_obj& dest_obj,
                rgw_obj& src_obj,
 	       time_t *mtime,
@@ -2815,7 +2834,7 @@ int RGWRados::copy_obj_data(void *ctx,
 
   do {
     bufferlist bl;
-    ret = get_obj(ctx, NULL, &handle, src_obj, bl, ofs, end);
+    ret = get_obj(ctx, NULL, handle, src_obj, bl, ofs, end);
     if (ret < 0)
       return ret;
 
@@ -2862,12 +2881,9 @@ int RGWRados::copy_obj_data(void *ctx,
   if (mtime)
     obj_stat(ctx, dest_obj, NULL, mtime, NULL, NULL, NULL, NULL);
 
-  finish_get_obj(&handle);
-
   return ret;
 done_err:
   delete_obj(ctx, shadow_obj);
-  finish_get_obj(&handle);
   return r;
 }
 
diff --git a/src/rgw/rgw_rados.h b/src/rgw/rgw_rados.h
index e6ab244afa9..e083390449b 100644
--- a/src/rgw/rgw_rados.h
+++ b/src/rgw/rgw_rados.h
@@ -1127,7 +1127,7 @@ public:
                void *progress_data);
 
   int copy_obj_data(void *ctx,
-	       void *handle, off_t end,
+	       void **handle, off_t end,
                rgw_obj& dest_obj,
                rgw_obj& src_obj,
 	       time_t *mtime,