From 8638a13918d1516d571d940aba1ccb27d55cb91d Mon Sep 17 00:00:00 2001
From: Daniel Patterson <danpat@danpat.net>
Date: Tue, 29 Sep 2020 16:52:53 -0600
Subject: [PATCH] Wrap access to facade_factory in a shared lock so it doesn't
 get changed partway through access which leads to a crash.

---
 CHANGELOG.md                     |  1 +
 include/engine/data_watchdog.hpp | 30 ++++++++++++++++++++----------
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c44f9ab8e..7a489880b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -17,6 +17,7 @@
       - CHANGED: default car weight was reduced to 2000 kg. [#5371](https://github.com/Project-OSRM/osrm-backend/pull/5371)
       - CHANGED: default car height was reduced to 2 meters. [#5389](https://github.com/Project-OSRM/osrm-backend/pull/5389)
       - FIXED: treat `bicycle=use_sidepath` as no access on the tagged way. [#5622](https://github.com/Project-OSRM/osrm-backend/pull/5622)
+      - FIXED: fix occasional segfault when swapping data with osrm-datastore and using `exclude=` [#5844](https://github.com/Project-OSRM/osrm-backend/pull/5844)
     - Misc:
       - CHANGED: Reduce memory usage for raster source handling. [#5572](https://github.com/Project-OSRM/osrm-backend/pull/5572)
       - CHANGED: Add cmake option `ENABLE_DEBUG_LOGGING` to control whether output debug logging. [#3427](https://github.com/Project-OSRM/osrm-backend/issues/3427)
diff --git a/include/engine/data_watchdog.hpp b/include/engine/data_watchdog.hpp
index b44274545..0f65ed10f 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 =
-                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,10 +78,14 @@ class DataWatchdogImpl<AlgorithmT, datafacade::ContiguousInternalMemoryDataFacad
 
     std::shared_ptr<const Facade> Get(const api::BaseParameters &params) const
     {
+        // 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
     {
+        // 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);
     }
 
@@ -111,16 +118,19 @@ class DataWatchdogImpl<AlgorithmT, datafacade::ContiguousInternalMemoryDataFacad
                         << (int)updatable_region.shm_key << " with timestamps "
                         << static_region.timestamp << " and " << updatable_region.timestamp;
 
-            facade_factory =
-                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;