Adds a shared/exclusive lock around queries and CheckAndReloadFacade.

Without this, it's possible for CheckAndReloadFacade to start working
while a query is still in progress, leading to undefined behaviour.
This commit is contained in:
Daniel Patterson 2016-01-15 00:57:36 -08:00 committed by Patrick Niklaus
parent 80b897d8cf
commit e21eaa4b9e
2 changed files with 73 additions and 42 deletions

View File

@ -223,6 +223,8 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
public: public:
virtual ~SharedDataFacade() {} virtual ~SharedDataFacade() {}
boost::shared_mutex data_mutex;
SharedDataFacade() SharedDataFacade()
{ {
if (!datastore::SharedMemory::RegionExists(CURRENT_REGIONS)) if (!datastore::SharedMemory::RegionExists(CURRENT_REGIONS))
@ -242,6 +244,15 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
void CheckAndReloadFacade() void CheckAndReloadFacade()
{ {
if (CURRENT_LAYOUT != data_timestamp_ptr->layout ||
CURRENT_DATA != data_timestamp_ptr->data ||
CURRENT_TIMESTAMP != data_timestamp_ptr->timestamp)
{
// Get exclusive lock
util::SimpleLogger().Write(logDEBUG) << "Updates available, getting exclusive lock";
boost::unique_lock<boost::shared_mutex> lock(data_mutex);
if (CURRENT_LAYOUT != data_timestamp_ptr->layout || if (CURRENT_LAYOUT != data_timestamp_ptr->layout ||
CURRENT_DATA != data_timestamp_ptr->data) CURRENT_DATA != data_timestamp_ptr->data)
{ {
@ -252,11 +263,20 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
CURRENT_LAYOUT = data_timestamp_ptr->layout; CURRENT_LAYOUT = data_timestamp_ptr->layout;
CURRENT_DATA = data_timestamp_ptr->data; CURRENT_DATA = data_timestamp_ptr->data;
CURRENT_TIMESTAMP = 0; // Force trigger a reload CURRENT_TIMESTAMP = 0; // Force trigger a reload
util::SimpleLogger().Write(logDEBUG) << "Current layout was different to new layout, swapping";
}
else
{
util::SimpleLogger().Write(logDEBUG) << "Current layout was same to new layout, not swapping";
} }
if (CURRENT_TIMESTAMP != data_timestamp_ptr->timestamp) if (CURRENT_TIMESTAMP != data_timestamp_ptr->timestamp)
{ {
CURRENT_TIMESTAMP = data_timestamp_ptr->timestamp; CURRENT_TIMESTAMP = data_timestamp_ptr->timestamp;
util::SimpleLogger().Write(logDEBUG) << "Performing data reload";
m_layout_memory.reset(datastore::SharedMemoryFactory::Get(CURRENT_LAYOUT)); m_layout_memory.reset(datastore::SharedMemoryFactory::Get(CURRENT_LAYOUT));
data_layout = (SharedDataLayout *) (m_layout_memory->Ptr()); data_layout = (SharedDataLayout *) (m_layout_memory->Ptr());
@ -267,8 +287,7 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
const char *file_index_ptr = const char *file_index_ptr =
data_layout->GetBlockPtr<char>(shared_memory, SharedDataLayout::FILE_INDEX_PATH); data_layout->GetBlockPtr<char>(shared_memory, SharedDataLayout::FILE_INDEX_PATH);
file_index_path = boost::filesystem::path(file_index_ptr); file_index_path = boost::filesystem::path(file_index_ptr);
if (!boost::filesystem::exists(file_index_path)) if (!boost::filesystem::exists(file_index_path)) {
{
util::SimpleLogger().Write(logDEBUG) << "Leaf file name " util::SimpleLogger().Write(logDEBUG) << "Leaf file name "
<< file_index_path.string(); << file_index_path.string();
throw util::exception("Could not load leaf index file. " throw util::exception("Could not load leaf index file. "
@ -295,6 +314,8 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
} }
} }
} }
util::SimpleLogger().Write(logDEBUG) << "Releasing exclusive lock";
}
} }
// search graph access // search graph access

View File

@ -82,8 +82,18 @@ int OSRM::OSRM_impl::RunQuery(const RouteParameters &route_parameters,
return 400; return 400;
} }
osrm::engine::plugins::BasePlugin::Status return_code;
increase_concurrent_query_count(); increase_concurrent_query_count();
auto return_code = plugin_iterator->second->HandleRequest(route_parameters, json_result); if (barrier) {
// Get a shared data lock so that other threads won't update
// things while the query is running
boost::shared_lock<boost::shared_mutex> data_lock{
(static_cast<datafacade::SharedDataFacade<contractor::QueryEdge::EdgeData> *>(
query_data_facade))->data_mutex};
return_code = plugin_iterator->second->HandleRequest(route_parameters, json_result);
} else {
return_code = plugin_iterator->second->HandleRequest(route_parameters, json_result);
}
decrease_concurrent_query_count(); decrease_concurrent_query_count();
return static_cast<int>(return_code); return static_cast<int>(return_code);
} }