diff --git a/src/iocore/cache/PreservationTable.cc b/src/iocore/cache/PreservationTable.cc index c45deb2d953..572e9af74f9 100644 --- a/src/iocore/cache/PreservationTable.cc +++ b/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 +#include 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); + this->evacuate = static_cast *>(ats_malloc(evac_len)); + memset(static_cast(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 { diff --git a/src/iocore/cache/PreservationTable.h b/src/iocore/cache/PreservationTable.h index 7641456b457..acf762cce97 100644 --- a/src/iocore/cache/PreservationTable.h +++ b/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 *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. * diff --git a/src/iocore/cache/StripeSM.cc b/src/iocore/cache/StripeSM.cc index d3c901ec59b..d5899dbe293 100644 --- a/src/iocore/cache/StripeSM.cc +++ b/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(len)} { - this->_preserved_dirs.evacuate_size = static_cast(len / EVACUATION_BUCKET_SIZE) + 2; - int evac_len = this->_preserved_dirs.evacuate_size * sizeof(DLL); - this->_preserved_dirs.evacuate = static_cast *>(ats_malloc(evac_len)); - memset(static_cast(this->_preserved_dirs.evacuate), 0, evac_len); - open_dir.mutex = this->mutex; SET_HANDLER(&StripeSM::aggWrite); } diff --git a/src/iocore/cache/unit_tests/test_Stripe.cc b/src/iocore/cache/unit_tests/test_Stripe.cc index 6c09b1af00c..dd81dcc8bbe 100644 --- a/src/iocore/cache/unit_tests/test_Stripe.cc +++ b/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); }