rgw: destroy get_obj handle in copy_obj()

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().

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>
Signed-off-by: Yehuda Sadeh <yehuda@inktank.com>
(cherry picked from commit 9e98620e4325d15c88440a890b267131613e1aa1)

diff --git a/src/rgw/rgw_rados.cc b/src/rgw/rgw_rados.cc
index 5ec61b965a12903e16e53f9b81b3ee870b4c1a9c..3db7c719a826114aa35a9c56e44b34628f3e594b 100644 (file)
--- a/src/rgw/rgw_rados.cc
+++ b/src/rgw/rgw_rados.cc
@@ -2493,6 +2493,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
@@ -2547,6 +2563,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;
@@ -2556,6 +2573,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 */
 
@@ -2699,7 +2718,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();
@@ -2804,7 +2823,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,
@@ -2830,7 +2849,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;
 
@@ -2877,12 +2896,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 a55f1c1f94cf4c80f5738b2a8f576b5a40667102..65765c414aaa8dd64889956af5dd074e345a40d6 100644 (file)
--- a/src/rgw/rgw_rados.h
+++ b/src/rgw/rgw_rados.h
@@ -1130,7 +1130,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,