From c4b3cdfd80067a99f62bc1d1c8283ab8f6f28e5a Mon Sep 17 00:00:00 2001 From: Patrick Niklaus Date: Fri, 16 Dec 2016 22:50:17 +0100 Subject: [PATCH] Fix changing shared memory in multi-process setup (#3462) This change fixes two bugs: 1. A dead-lock that occurs between osrm-datastore and libosrm when an old dataset is free during a data update. This happened because the mutexes where acquired in a different order. 2. A region is deleted eventhough it is still in use. This happens when libosrm gets overtaken by osrm-datastore, so the new dataset is in the same region the old one was. --- .../datafacade/shared_memory_datafacade.hpp | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/include/engine/datafacade/shared_memory_datafacade.hpp b/include/engine/datafacade/shared_memory_datafacade.hpp index 72eb5b916..c6fb0bbf9 100644 --- a/include/engine/datafacade/shared_memory_datafacade.hpp +++ b/include/engine/datafacade/shared_memory_datafacade.hpp @@ -38,29 +38,33 @@ class SharedMemoryDataFacade : public ContiguousInternalMemoryDataFacadeBase // used anymore. We crash hard here if something goes wrong (noexcept). virtual ~SharedMemoryDataFacade() noexcept { + // Now check if this is still the newest dataset + boost::interprocess::sharable_lock + current_regions_lock(shared_barriers->current_regions_mutex, + boost::interprocess::defer_lock); + boost::interprocess::scoped_lock exclusive_lock( data_region == storage::DATA_1 ? shared_barriers->regions_1_mutex : shared_barriers->regions_2_mutex, boost::interprocess::defer_lock); // if this returns false this is still in use - if (exclusive_lock.try_lock()) + if (current_regions_lock.try_lock() && exclusive_lock.try_lock()) { if (storage::SharedMemory::RegionExists(data_region)) { BOOST_ASSERT(storage::SharedMemory::RegionExists(layout_region)); - // Now check if this is still the newest dataset - const boost::interprocess::sharable_lock - lock(shared_barriers->current_regions_mutex); - auto shared_regions = storage::makeSharedMemory(storage::CURRENT_REGIONS); const auto current_timestamp = static_cast(shared_regions->Ptr()); - if (current_timestamp->timestamp == shared_timestamp) + // check if the memory region referenced by this facade needs cleanup + if (current_timestamp->data == data_region) { - util::Log(logDEBUG) << "Retaining data with shared timestamp " << shared_timestamp; + BOOST_ASSERT(current_timestamp->layout == layout_region); + util::Log(logDEBUG) << "Retaining data with shared timestamp " + << shared_timestamp; } else {