Implement RAII for PreservationTable

src/iocore/cache/PreservationTable.cc

@@ -31,7 +31,12 @@
 
 #include "tsutil/DbgCtl.h"
 
+#include "tscore/ink_assert.h"
+#include "tscore/ink_memory.h"
+#include "tscore/List.h"
+
 #include <cinttypes>
+#include <cstring>
 
 namespace
 {
@@ -40,6 +45,38 @@ DbgCtl dbg_ctl_cache_evac{"cache_evac"};
 
 } // namespace
 
+PreservationTable::PreservationTable(int size) : evacuate_size{(size / EVACUATION_BUCKET_SIZE) + 2}
+{
+  ink_assert(size > 0);
+  int evac_len   = this->evacuate_size * sizeof(DLL<EvacuationBlock>);
+  this->evacuate = static_cast<DLL<EvacuationBlock> *>(ats_malloc(evac_len));
+  memset(static_cast<void *>(this->evacuate), 0, evac_len);
+}
+
+PreservationTable::~PreservationTable()
+{
+  ats_free(this->evacuate);
+  this->evacuate = nullptr;
+}
+
+PreservationTable::PreservationTable(PreservationTable &&that)
+{
+  this->evacuate_size = that.evacuate_size;
+  this->evacuate      = that.evacuate;
+  that.evacuate_size  = 0;
+  that.evacuate       = nullptr;
+}
+
+PreservationTable &
+PreservationTable::operator=(PreservationTable &&that)
+{
+  this->evacuate_size = that.evacuate_size;
+  this->evacuate      = that.evacuate;
+  that.evacuate_size  = 0;
+  that.evacuate       = nullptr;
+  return *this;
+}
+
 EvacuationBlock *
 PreservationTable::find(Dir const &dir) const
 {

src/iocore/cache/PreservationTable.h

@@ -36,8 +36,8 @@
 #include "tscore/ink_platform.h"
 #include "tscore/List.h"
 
-#define EVACUATION_BUCKET_SIZE (2 * EVACUATION_SIZE) // 16MB
 #define EVACUATION_SIZE        (2 * AGG_SIZE)        // 8MB
+#define EVACUATION_BUCKET_SIZE (2 * EVACUATION_SIZE) // 16MB
 #define PIN_SCAN_EVERY         16                    // scan every 1/16 of disk
 
 #define dir_offset_evac_bucket(_o) (_o / (EVACUATION_BUCKET_SIZE / CACHE_BLOCK_SIZE))
@@ -85,6 +85,9 @@ struct EvacuationBlock {
  * This class is not safe for concurrent access. It should be protected
  * by a lock.
  *
+ * Destructing this object without first releasing all blocks and calling
+ * periodic_scan may result in memory leaks.
+ *
  * @see Stripe
  */
 class PreservationTable
@@ -99,6 +102,24 @@ class PreservationTable
    */
   DLL<EvacuationBlock> *evacuate{nullptr};
 
+  /**
+   * PreservationTable constructor
+   *
+   * This will abort the program if it is unable to allocate memory.
+   *
+   * @param size The total number of directory entries the table must
+   * be able to store. The size must be a positive number.
+   */
+  PreservationTable(int size);
+
+  ~PreservationTable();
+
+  PreservationTable(PreservationTable const &that)            = delete;
+  PreservationTable &operator=(PreservationTable const &that) = delete;
+
+  PreservationTable(PreservationTable &&that);
+  PreservationTable &operator=(PreservationTable &&that);
+
   /**
    * Check whether the hash table may be indexed with the given offset.
    *

src/iocore/cache/StripeSM.cc

@@ -109,13 +109,12 @@ struct StripeInitInfo {
   }
 };
 
-StripeSM::StripeSM(CacheDisk *disk, off_t blocks, off_t dir_skip) : Continuation(new_ProxyMutex()), Stripe{disk, blocks, dir_skip}
+// This is weird: the len passed to the constructor for _preserved_dirs is
+// initialized in the superclasse's constructor. This is safe because the
+// superclass should always be initialized first.
+StripeSM::StripeSM(CacheDisk *disk, off_t blocks, off_t dir_skip)
+  : Continuation(new_ProxyMutex()), Stripe{disk, blocks, dir_skip}, _preserved_dirs{static_cast<int>(len)}
 {
-  this->_preserved_dirs.evacuate_size = static_cast<int>(len / EVACUATION_BUCKET_SIZE) + 2;
-  int evac_len                        = this->_preserved_dirs.evacuate_size * sizeof(DLL<EvacuationBlock>);
-  this->_preserved_dirs.evacuate      = static_cast<DLL<EvacuationBlock> *>(ats_malloc(evac_len));
-  memset(static_cast<void *>(this->_preserved_dirs.evacuate), 0, evac_len);
-
   open_dir.mutex = this->mutex;
   SET_HANDLER(&StripeSM::aggWrite);
 }

src/iocore/cache/unit_tests/test_Stripe.cc

@@ -193,7 +193,6 @@ TEST_CASE("The behavior of StripeSM::add_writer.")
   }
 
   ats_free(stripe.raw_dir);
-  ats_free(stripe.get_preserved_dirs().evacuate);
   ats_free(stripe.path);
 }
 
@@ -305,7 +304,6 @@ TEST_CASE("aggWrite behavior with f.evacuator unset")
   }
 
   ats_free(stripe.raw_dir);
-  ats_free(stripe.get_preserved_dirs().evacuate);
   ats_free(stripe.path);
 }
 
@@ -405,6 +403,5 @@ TEST_CASE("aggWrite behavior with f.evacuator set")
 
   delete[] source;
   ats_free(stripe.raw_dir);
-  ats_free(stripe.get_preserved_dirs().evacuate);
   ats_free(stripe.path);
 }