From ba78fa82a2e52b550f54324641a5f7602fc549f5 Mon Sep 17 00:00:00 2001
From: Daniel Patterson <danpat@danpat.net>
Date: Wed, 30 Sep 2020 08:36:29 -0600
Subject: [PATCH] Wrap access to facade_factory in a shared lock so it doesn't
 get changed partway through access, leading to a crash.

---
 include/engine/data_watchdog.hpp | 46 ++++++++++++++++----------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/include/engine/data_watchdog.hpp b/include/engine/data_watchdog.hpp
index b71ab080325..0f65ed10f65 100644
--- a/include/engine/data_watchdog.hpp
+++ b/include/engine/data_watchdog.hpp
@@ -56,11 +56,14 @@ class DataWatchdogImpl<AlgorithmT, datafacade::ContiguousInternalMemoryDataFacad
             static_region = *static_shared_region;
             updatable_region = *updatable_shared_region;
 
-            facade_factory =
-                std::make_shared<DataFacadeFactory<datafacade::ContiguousInternalMemoryDataFacade, AlgorithmT>>(
-                    std::make_shared<datafacade::SharedMemoryAllocator>(
-                        std::vector<storage::SharedRegionRegister::ShmKey>{
-                            static_region.shm_key, updatable_region.shm_key}));
+            {
+                boost::unique_lock<boost::shared_mutex> swap_lock(factory_mutex);
+                facade_factory =
+                    DataFacadeFactory<datafacade::ContiguousInternalMemoryDataFacade, AlgorithmT>(
+                        std::make_shared<datafacade::SharedMemoryAllocator>(
+                            std::vector<storage::SharedRegionRegister::ShmKey>{
+                                static_region.shm_key, updatable_region.shm_key}));
+            }
         }
 
         watcher = std::thread(&DataWatchdogImpl::Run, this);
@@ -75,21 +78,15 @@ class DataWatchdogImpl<AlgorithmT, datafacade::ContiguousInternalMemoryDataFacad
 
     std::shared_ptr<const Facade> Get(const api::BaseParameters &params) const
     {
-        // Ensure that an exchange of facade_factory while ->Get() is in progress
-        // doesn't access an object undergoing destruction from the Run() thread
-        // We increase the ref count on facade_factory, and it only gets decreased
-        // (possibly to zero) *after* this function returns
-        auto facade_factory_copy = facade_factory;
-        return facade_factory_copy->Get(params);
+        // make sure facade_factory stays stable while we call Get()
+        boost::shared_lock<boost::shared_mutex> swap_lock(factory_mutex);
+        return facade_factory.Get(params);
     }
     std::shared_ptr<const Facade> Get(const api::TileParameters &params) const
     {
-        // Ensure that an exchange of facade_factory while ->Get() is in progress
-        // doesn't access an object undergoing destruction from the Run() thread
-        // We increase the ref count on facade_factory, and it only gets decreased
-        // (possibly to zero) *after* this function returns
-        auto facade_factory_copy = facade_factory;
-        return facade_factory_copy->Get(params);
+        // make sure facade_factory stays stable while we call Get()
+        boost::shared_lock<boost::shared_mutex> swap_lock(factory_mutex);
+        return facade_factory.Get(params);
     }
 
   private:
@@ -121,16 +118,19 @@ class DataWatchdogImpl<AlgorithmT, datafacade::ContiguousInternalMemoryDataFacad
                         << (int)updatable_region.shm_key << " with timestamps "
                         << static_region.timestamp << " and " << updatable_region.timestamp;
 
-            facade_factory =
-                std::make_shared<DataFacadeFactory<datafacade::ContiguousInternalMemoryDataFacade, AlgorithmT>>(
-                    std::make_shared<datafacade::SharedMemoryAllocator>(
-                        std::vector<storage::SharedRegionRegister::ShmKey>{
-                            static_region.shm_key, updatable_region.shm_key}));
+            {
+                boost::unique_lock<boost::shared_mutex> swap_lock(factory_mutex);
+                facade_factory =DataFacadeFactory<datafacade::ContiguousInternalMemoryDataFacade, AlgorithmT>(
+                        std::make_shared<datafacade::SharedMemoryAllocator>(
+                            std::vector<storage::SharedRegionRegister::ShmKey>{
+                                static_region.shm_key, updatable_region.shm_key}));
+            }
         }
 
         util::Log() << "DataWatchdog thread stopped";
     }
 
+    mutable boost::shared_mutex factory_mutex;
     const std::string dataset_name;
     storage::SharedMonitor<storage::SharedRegionRegister> barrier;
     std::thread watcher;
@@ -139,7 +139,7 @@ class DataWatchdogImpl<AlgorithmT, datafacade::ContiguousInternalMemoryDataFacad
     storage::SharedRegion updatable_region;
     storage::SharedRegion *static_shared_region;
     storage::SharedRegion *updatable_shared_region;
-    std::shared_ptr<DataFacadeFactory<datafacade::ContiguousInternalMemoryDataFacade, AlgorithmT>> facade_factory;
+    DataFacadeFactory<datafacade::ContiguousInternalMemoryDataFacade, AlgorithmT> facade_factory;
 };
 }