Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lock access to facade_factory in data_watchdog to avoid accessing destructed object #5844

Merged
merged 2 commits into from
Oct 1, 2020
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Wrap access to facade_factory in a shared lock so it doesn't get chan…
…ged partway through access which leads to a crash.
danpat committed Sep 30, 2020

Verified

This commit was signed with the committer’s verified signature.
danpat Daniel Patterson
commit 8638a13918d1516d571d940aba1ccb27d55cb91d
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)
30 changes: 20 additions & 10 deletions include/engine/data_watchdog.hpp
Original file line number Diff line number Diff line change
@@ -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;