Fix hosting.config reload

iocore/cache/Cache.cc

@@ -1975,13 +1975,16 @@ CacheProcessor::mark_storage_offline(CacheDisk *d, ///< Target disk
     Warning("All storage devices offline, cache disabled");
     CacheProcessor::cache_ready = 0;
   } else { // check cache types specifically
-    if (theCache && !theCache->hosttable->gen_host_rec.vol_hash_table) {
-      unsigned int caches_ready = 0;
-      caches_ready              = caches_ready | (1 << CACHE_FRAG_TYPE_HTTP);
-      caches_ready              = caches_ready | (1 << CACHE_FRAG_TYPE_NONE);
-      caches_ready              = ~caches_ready;
-      CacheProcessor::cache_ready &= caches_ready;
-      Warning("all volumes for http cache are corrupt, http cache disabled");
+    if (theCache) {
+      ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(&theCache->hosttable);
+      if (!hosttable->gen_host_rec.vol_hash_table) {
+        unsigned int caches_ready = 0;
+        caches_ready              = caches_ready | (1 << CACHE_FRAG_TYPE_HTTP);
+        caches_ready              = caches_ready | (1 << CACHE_FRAG_TYPE_NONE);
+        caches_ready              = ~caches_ready;
+        CacheProcessor::cache_ready &= caches_ready;
+        Warning("all volumes for http cache are corrupt, http cache disabled");
+      }
     }
   }
 
@@ -2050,9 +2053,13 @@ Cache::open_done()
     return 0;
   }
 
-  hosttable = new CacheHostTable(this, scheme);
-  hosttable->register_config_callback(&hosttable);
+  {
+    CacheHostTable *hosttable_raw = new CacheHostTable(this, scheme);
+    hosttable.reset(hosttable_raw);
+    hosttable_raw->register_config_callback(&hosttable);
+  }
 
+  ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(&this->hosttable);
   if (hosttable->gen_host_rec.num_cachevols == 0) {
     ready = CACHE_INIT_FAILED;
   } else {
@@ -3006,9 +3013,10 @@ create_volume(int volume_number, off_t size_in_blocks, int scheme, CacheVol *cp)
 void
 rebuild_host_table(Cache *cache)
 {
-  build_vol_hash_table(&cache->hosttable->gen_host_rec);
-  if (cache->hosttable->m_numEntries != 0) {
-    CacheHostMatcher *hm   = cache->hosttable->getHostMatcher();
+  ReplaceablePtr<CacheHostTable>::ScopedWriter hosttable(&cache->hosttable);
+  build_vol_hash_table(&hosttable->gen_host_rec);
+  if (hosttable->m_numEntries != 0) {
+    CacheHostMatcher *hm   = hosttable->getHostMatcher();
     CacheHostRecord *h_rec = hm->getDataArray();
     int h_rec_len          = hm->getNumElements();
     int i;
@@ -3022,9 +3030,11 @@ rebuild_host_table(Cache *cache)
 Vol *
 Cache::key_to_vol(const CacheKey *key, const char *hostname, int host_len)
 {
-  uint32_t h                 = (key->slice32(2) >> DIR_TAG_WIDTH) % VOL_HASH_TABLE_SIZE;
-  unsigned short *hash_table = hosttable->gen_host_rec.vol_hash_table;
-  CacheHostRecord *host_rec  = &hosttable->gen_host_rec;
+  ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(&this->hosttable);
+
+  uint32_t h                      = (key->slice32(2) >> DIR_TAG_WIDTH) % VOL_HASH_TABLE_SIZE;
+  unsigned short *hash_table      = hosttable->gen_host_rec.vol_hash_table;
+  const CacheHostRecord *host_rec = &hosttable->gen_host_rec;
 
   if (hosttable->m_numEntries > 0 && host_len) {
     CacheHostResult res;

iocore/cache/CacheHosting.cc

@@ -229,7 +229,7 @@ int
 CacheHostTable::config_callback(const char * /* name ATS_UNUSED */, RecDataT /* data_type ATS_UNUSED */,
                                 RecData /* data ATS_UNUSED */, void *cookie)
 {
-  CacheHostTable **ppt = static_cast<CacheHostTable **>(cookie);
+  ReplaceablePtr<CacheHostTable> *ppt = static_cast<ReplaceablePtr<CacheHostTable> *>(cookie);
   eventProcessor.schedule_imm(new CacheHostTableConfig(ppt));
   return 0;
 }

iocore/cache/CacheVol.cc

@@ -57,14 +57,18 @@ CacheVC::scanVol(int /* event ATS_UNUSED */, Event * /* e ATS_UNUSED */)
   if (_action.cancelled) {
     return free_CacheVC(this);
   }
-  CacheHostRecord *rec = &theCache->hosttable->gen_host_rec;
+
+  ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(&theCache->hosttable);
+
+  const CacheHostRecord *rec = &hosttable->gen_host_rec;
   if (host_len) {
     CacheHostResult res;
-    theCache->hosttable->Match(hostname, host_len, &res);
+    hosttable->Match(hostname, host_len, &res);
     if (res.record) {
       rec = res.record;
     }
   }
+
   if (!vol) {
     if (!rec->num_vols) {
       goto Ldone;

iocore/cache/P_CacheHosting.h

@@ -22,6 +22,8 @@
  */
 
 #pragma once
+#include <memory>
+#include <shared_mutex>
 #include "P_Cache.h"
 #include "tscore/MatcherUtils.h"
 #include "tscore/HostLookup.h"
@@ -105,6 +107,113 @@ class CacheHostMatcher
   CacheType type;
 };
 
+// ReplaceablePtr provides threadsafe access to an object which may be replaced.
+//
+// Access is provided via ScopedReader and ScopedWriter classes, which acquire
+// shared (read) and exclusive (write) locks respectively on construction and
+// release them upon destruction.
+//
+// The underlying object may be replaced by a concurrent thread via 'reset',
+// which acquires an exclusive lock before setting the internal pointer. This
+// takes ownership of the given pointer. If this already has ownership of
+// another pointer, that object is destroyed and its memory freed.
+//
+// Direct access without acquiring the lock is intentionally not provided, to
+// prevent accidental usage without locking, or forgetting to release the lock.
+//
+// This may not be copied. To use, construct with one owner, and pass a pointer
+// to other users.
+//
+template <typename T> class ReplaceablePtr
+{
+public:
+  ReplaceablePtr() {}
+  virtual ~ReplaceablePtr() {}
+
+  // reset acquires an exclusive (write) lock and updates the internal pointer
+  // with t.
+  // If an existing pointer is owned, the object is destructed and its memory
+  // freed.
+  void
+  reset(T *t)
+  {
+    std::scoped_lock l(m);
+    h.reset(t);
+  }
+
+  // ScopedReader constructs an object which is allowed to read from a
+  // ReplaceablePtr.
+  //
+  // The lifetime of the ReplaceablePtr must exceed the lifetime of this.
+  // The shared (read) lock is immediately acquired  upon construction of this,
+  // and released upon destruction.
+  class ScopedReader
+  {
+  public:
+    ScopedReader(ReplaceablePtr<T> *ptr) : ptr(ptr) { ptr->m.lock_shared(); }
+    ~ScopedReader() { ptr->m.unlock_shared(); }
+
+    const T *
+    operator->()
+    {
+      return ptr->h.get();
+    }
+
+    const T *
+    get()
+    {
+      return ptr->h.get();
+    }
+
+  private:
+    ScopedReader(const ScopedReader &) = delete;
+    ScopedReader &operator=(const ScopedReader &) = delete;
+
+    ReplaceablePtr<T> *ptr;
+  };
+
+  // ScopedWriter constructs an object which is allowed to read and modify the
+  // object pointed to by a ReplaceablePtr.
+  //
+  // The lifetime of the ReplaceablePtr must exceed the lifetime of this.
+  //
+  // An exclusive (write) lock is immediately acquired  upon construction of
+  // this, and released upon destruction.
+  class ScopedWriter
+  {
+  public:
+    ScopedWriter(ReplaceablePtr<T> *ptr) : ptr(ptr) { ptr->m.lock(); }
+    ~ScopedWriter() { ptr->m.unlock(); }
+
+    T *
+    operator->()
+    {
+      return ptr->h.get();
+    }
+
+    T *
+    get()
+    {
+      return ptr->h.get();
+    }
+
+  private:
+    ScopedWriter(const ScopedWriter &) = delete;
+    ScopedWriter &operator=(const ScopedWriter &) = delete;
+
+    ReplaceablePtr<T> *ptr;
+  };
+
+private:
+  ReplaceablePtr(const ReplaceablePtr &) = delete;
+  ReplaceablePtr &operator=(const ReplaceablePtr &) = delete;
+
+  std::unique_ptr<T> h = nullptr;
+  std::shared_mutex m;
+
+  friend class ReplaceablePtr::ScopedReader;
+};
+
 class CacheHostTable
 {
 public:
@@ -133,7 +242,7 @@ class CacheHostTable
   static int config_callback(const char *, RecDataT, RecData, void *);
 
   void
-  register_config_callback(CacheHostTable **p)
+  register_config_callback(ReplaceablePtr<CacheHostTable> *p)
   {
     REC_RegisterConfigUpdateFunc("proxy.config.cache.hosting_filename", CacheHostTable::config_callback, (void *)p);
   }
@@ -152,20 +261,33 @@ class CacheHostTable
 struct CacheHostTableConfig;
 typedef int (CacheHostTableConfig::*CacheHostTabHandler)(int, void *);
 struct CacheHostTableConfig : public Continuation {
-  CacheHostTable **ppt;
-  CacheHostTableConfig(CacheHostTable **appt) : Continuation(nullptr), ppt(appt) { SET_HANDLER(&CacheHostTableConfig::mainEvent); }
+  CacheHostTableConfig(ReplaceablePtr<CacheHostTable> *appt) : Continuation(nullptr), ppt(appt)
+  {
+    SET_HANDLER(&CacheHostTableConfig::mainEvent);
+  }
+
+  ~CacheHostTableConfig() {}
 
   int
   mainEvent(int event, Event *e)
   {
     (void)e;
     (void)event;
-    CacheHostTable *t   = new CacheHostTable((*ppt)->cache, (*ppt)->type);
-    CacheHostTable *old = (CacheHostTable *)ink_atomic_swap(&t, *ppt);
-    new_Deleter(old, CACHE_MEM_FREE_TIMEOUT);
+
+    CacheType type = CACHE_HTTP_TYPE;
+    Cache *cache   = nullptr;
+    {
+      ReplaceablePtr<CacheHostTable>::ScopedReader hosttable(ppt);
+      type  = hosttable->type;
+      cache = hosttable->cache;
+    }
+    ppt->reset(new CacheHostTable(cache, type));
     delete this;
     return EVENT_DONE;
   }
+
+private:
+  ReplaceablePtr<CacheHostTable> *ppt;
 };
 
 /* list of volumes in the volume.config file */

iocore/cache/P_CacheInternal.h

@@ -28,6 +28,7 @@
 
 #include "HTTP.h"
 #include "P_CacheHttp.h"
+#include "P_CacheHosting.h"
 
 struct EvacuationBlock;
 
@@ -987,10 +988,11 @@ struct Cache {
   int total_nvol            = 0;
   int ready                 = CACHE_INITIALIZING;
   int64_t cache_size        = 0; // in store block size
-  CacheHostTable *hosttable = nullptr;
   int total_initialized_vol = 0;
   CacheType scheme          = CACHE_NONE_TYPE;
 
+  ReplaceablePtr<CacheHostTable> hosttable;
+
   int open(bool reconfigure, bool fix);
   int close();