rgw_sal_motr: fix possible memleak on put

Currently, the MotrAtomicWriter::cleanup() is called from
MotrAtomicWriter::commit(), which may not be called at all
by rgw in case of md5 checksum failure.

Solution: call cleanup() from process() when data is zero.
rgw calls Writer::process(data, off) with zero data at the
end of the loop to allow writes to flush the data. From:
src/rgw/rgw_op.cc:RGWPutObj::execute():

      op_ret = filter->process(std::move(data), ofs);
      ...
      ofs += len;
    } while (len > 0);

    // flush any data in filters
    op_ret = filter->process({}, ofs);

Signed-off-by: Andriy Tkachuk <andriy.tkachuk@seagate.com>
Reviewed-by: Sining Wu <sining.wu@seagate.com>

diff --git a/src/rgw/rgw_sal.h b/src/rgw/rgw_sal.h
index b164b3cbb736e7a087e60e0f6532ef6a208b5e7f..2fc8ab62b9bb7d3ebbaa1d84359be7417f0d668c 100644 (file)
--- a/src/rgw/rgw_sal.h
+++ b/src/rgw/rgw_sal.h
@@ -1369,7 +1369,11 @@ public:
   /** prepare to start processing object data */
   virtual int prepare(optional_yield y) = 0;
 
-  /** Process a buffer.  Called multiple times to write different buffers. */
+  /**
+   * Process a buffer. Called multiple times to write different buffers.
+   * data.length() == 0 indicates the last call and may be used to flush
+   * the data buffers.
+   */
   virtual int process(bufferlist&& data, uint64_t offset) = 0;
 
   /** complete the operation and make its result visible to clients */

diff --git a/src/rgw/rgw_sal_motr.cc b/src/rgw/rgw_sal_motr.cc
index 794ede6df286b676fa395f0a2a197ba7cd4498bc..379f91df82a58db83b9c070327014516b7054742 100644 (file)
--- a/src/rgw/rgw_sal_motr.cc
+++ b/src/rgw/rgw_sal_motr.cc
@@ -2023,8 +2023,13 @@ static const unsigned MAX_ACC_SIZE = 32 * 1024 * 1024;
 // and then launch the write operations.
 int MotrAtomicWriter::process(bufferlist&& data, uint64_t offset)
 {
-  if (data.length() == 0)
-    return 0;
+  if (data.length() == 0) { // last call, flush data
+    int rc = 0;
+    if (acc_data.length() != 0)
+      rc = this->write();
+    this->cleanup();
+    return rc;
+  }
 
   if (acc_data.length() == 0)
     acc_off = offset;
@@ -2047,13 +2052,12 @@ int MotrAtomicWriter::complete(size_t accounted_size, const std::string& etag,
 {
   int rc = 0;
 
-  if (acc_data.length() != 0)
+  if (acc_data.length() != 0) { // check again, just in case
     rc = this->write();
-
-  this->cleanup();
-
-  if (rc != 0)
-    return rc;
+    this->cleanup();
+    if (rc != 0)
+      return rc;
+  }
 
   bufferlist bl;
   rgw_bucket_dir_entry ent;
@@ -2113,7 +2117,7 @@ int MotrAtomicWriter::complete(size_t accounted_size, const std::string& etag,
     // TODO: update the current version (unset the flag) and insert the new current
     // version can be launched in one motr op. This requires change at do_idx_op()
     // and do_idx_op_by_name().
-    int rc = obj.update_version_entries(dpp);
+    rc = obj.update_version_entries(dpp);
     if (rc < 0)
       return rc;
   }