Lock access to facade_factory in data_watchdog to avoid accessing des…

…tructed object (#5844) * Wrap access to facade_factory in a shared lock so it doesn't get changed partway through access which leads to a crash.
Project-OSRM · Oct 1, 2020 · 3451d1e · 3451d1e
1 parent 4799b46
commit 3451d1e
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 10 deletions.
diff --git 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
@@ -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,20 @@ 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;