Skip to content

Commit d188e8e

Browse files
Patrick Niklausdanpat
authored andcommitted
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.
1 parent f88f51f commit d188e8e

File tree

1 file changed

+11
-7
lines changed

1 file changed

+11
-7
lines changed

include/engine/datafacade/shared_memory_datafacade.hpp

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -38,29 +38,33 @@ class SharedMemoryDataFacade : public ContiguousInternalMemoryDataFacadeBase
3838
// used anymore. We crash hard here if something goes wrong (noexcept).
3939
virtual ~SharedMemoryDataFacade() noexcept
4040
{
41+
// Now check if this is still the newest dataset
42+
boost::interprocess::sharable_lock<boost::interprocess::named_upgradable_mutex>
43+
current_regions_lock(shared_barriers->current_regions_mutex,
44+
boost::interprocess::defer_lock);
45+
4146
boost::interprocess::scoped_lock<boost::interprocess::named_sharable_mutex> exclusive_lock(
4247
data_region == storage::DATA_1 ? shared_barriers->regions_1_mutex
4348
: shared_barriers->regions_2_mutex,
4449
boost::interprocess::defer_lock);
4550

4651
// if this returns false this is still in use
47-
if (exclusive_lock.try_lock())
52+
if (current_regions_lock.try_lock() && exclusive_lock.try_lock())
4853
{
4954
if (storage::SharedMemory::RegionExists(data_region))
5055
{
5156
BOOST_ASSERT(storage::SharedMemory::RegionExists(layout_region));
5257

53-
// Now check if this is still the newest dataset
54-
const boost::interprocess::sharable_lock<boost::interprocess::named_upgradable_mutex>
55-
lock(shared_barriers->current_regions_mutex);
56-
5758
auto shared_regions = storage::makeSharedMemory(storage::CURRENT_REGIONS);
5859
const auto current_timestamp =
5960
static_cast<const storage::SharedDataTimestamp *>(shared_regions->Ptr());
6061

61-
if (current_timestamp->timestamp == shared_timestamp)
62+
// check if the memory region referenced by this facade needs cleanup
63+
if (current_timestamp->data == data_region)
6264
{
63-
util::Log(logDEBUG) << "Retaining data with shared timestamp " << shared_timestamp;
65+
BOOST_ASSERT(current_timestamp->layout == layout_region);
66+
util::Log(logDEBUG) << "Retaining data with shared timestamp "
67+
<< shared_timestamp;
6468
}
6569
else
6670
{

0 commit comments

Comments
 (0)