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:
		
							parent
							
								
									25c8711aad
								
							
						
					
					
						commit
						23b2154d98
					
				| @ -109,7 +109,16 @@ int OSRM::OSRM_impl::RunQuery(const RouteParameters &route_parameters, osrm::jso | |||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|     increase_concurrent_query_count(); |     increase_concurrent_query_count(); | ||||||
|     auto return_code = plugin_iterator->second->HandleRequest(route_parameters, json_result); |     BasePlugin::Status return_code; | ||||||
|  |     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<SharedDataFacade<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); | ||||||
| } | } | ||||||
|  | |||||||
| @ -239,6 +239,8 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade< | |||||||
|   public: |   public: | ||||||
|     virtual ~SharedDataFacade() {} |     virtual ~SharedDataFacade() {} | ||||||
| 
 | 
 | ||||||
|  |     boost::shared_mutex data_mutex; | ||||||
|  | 
 | ||||||
|     SharedDataFacade() |     SharedDataFacade() | ||||||
|     { |     { | ||||||
|         if (!SharedMemory::RegionExists(CURRENT_REGIONS)) |         if (!SharedMemory::RegionExists(CURRENT_REGIONS)) | ||||||
| @ -259,56 +261,74 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade< | |||||||
|     void CheckAndReloadFacade() |     void CheckAndReloadFacade() | ||||||
|     { |     { | ||||||
|         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 || | ||||||
|  |             CURRENT_TIMESTAMP != data_timestamp_ptr->timestamp) | ||||||
|         { |         { | ||||||
|             // release the previous shared memory segments
 |             // Get exclusive lock
 | ||||||
|             SharedMemory::Remove(CURRENT_LAYOUT); |             SimpleLogger().Write(logDEBUG) << "Updates available, getting exclusive lock"; | ||||||
|             SharedMemory::Remove(CURRENT_DATA); |             boost::unique_lock<boost::shared_mutex> lock(data_mutex); | ||||||
| 
 | 
 | ||||||
|             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) | ||||||
|             CURRENT_TIMESTAMP = 0;  // Force trigger a reload
 |  | ||||||
|         } |  | ||||||
| 
 |  | ||||||
|         if (CURRENT_TIMESTAMP != data_timestamp_ptr->timestamp) |  | ||||||
|         { |  | ||||||
|             CURRENT_TIMESTAMP = data_timestamp_ptr->timestamp; |  | ||||||
|             m_layout_memory.reset(SharedMemoryFactory::Get(CURRENT_LAYOUT)); |  | ||||||
| 
 |  | ||||||
|             data_layout = (SharedDataLayout *)(m_layout_memory->Ptr()); |  | ||||||
| 
 |  | ||||||
|             m_large_memory.reset(SharedMemoryFactory::Get(CURRENT_DATA)); |  | ||||||
|             shared_memory = (char *)(m_large_memory->Ptr()); |  | ||||||
| 
 |  | ||||||
|             const char *file_index_ptr = |  | ||||||
|                 data_layout->GetBlockPtr<char>(shared_memory, SharedDataLayout::FILE_INDEX_PATH); |  | ||||||
|             file_index_path = boost::filesystem::path(file_index_ptr); |  | ||||||
|             if (!boost::filesystem::exists(file_index_path)) |  | ||||||
|             { |             { | ||||||
|                 SimpleLogger().Write(logDEBUG) << "Leaf file name " << file_index_path.string(); |                 // release the previous shared memory segments
 | ||||||
|                 throw osrm::exception("Could not load leaf index file. " |                 SharedMemory::Remove(CURRENT_LAYOUT); | ||||||
|                                       "Is any data loaded into shared memory?"); |                 SharedMemory::Remove(CURRENT_DATA); | ||||||
|  | 
 | ||||||
|  |                 CURRENT_LAYOUT = data_timestamp_ptr->layout; | ||||||
|  |                 CURRENT_DATA = data_timestamp_ptr->data; | ||||||
|  |                 CURRENT_TIMESTAMP = 0;  // Force trigger a reload
 | ||||||
|  | 
 | ||||||
|  |                 SimpleLogger().Write(logDEBUG) << "Current layout was different to new layout, swapping"; | ||||||
|  |             } | ||||||
|  |             else | ||||||
|  |             { | ||||||
|  |                 SimpleLogger().Write(logDEBUG) << "Current layout was same to new layout, not swapping"; | ||||||
|             } |             } | ||||||
| 
 | 
 | ||||||
|             LoadGraph(); |             if (CURRENT_TIMESTAMP != data_timestamp_ptr->timestamp) | ||||||
|             LoadChecksum(); |  | ||||||
|             LoadNodeAndEdgeInformation(); |  | ||||||
|             LoadGeometries(); |  | ||||||
|             LoadTimestamp(); |  | ||||||
|             LoadViaNodeList(); |  | ||||||
|             LoadNames(); |  | ||||||
|             LoadCoreInformation(); |  | ||||||
| 
 |  | ||||||
|             data_layout->PrintInformation(); |  | ||||||
| 
 |  | ||||||
|             SimpleLogger().Write() << "number of geometries: " << m_coordinate_list->size(); |  | ||||||
|             for (unsigned i = 0; i < m_coordinate_list->size(); ++i) |  | ||||||
|             { |             { | ||||||
|                 if (!GetCoordinateOfNode(i).is_valid()) |                 CURRENT_TIMESTAMP = data_timestamp_ptr->timestamp; | ||||||
|  | 
 | ||||||
|  |                 SimpleLogger().Write(logDEBUG) << "Performing data reload"; | ||||||
|  |                 m_layout_memory.reset(SharedMemoryFactory::Get(CURRENT_LAYOUT)); | ||||||
|  | 
 | ||||||
|  |                 data_layout = (SharedDataLayout *) (m_layout_memory->Ptr()); | ||||||
|  | 
 | ||||||
|  |                 m_large_memory.reset(SharedMemoryFactory::Get(CURRENT_DATA)); | ||||||
|  |                 shared_memory = (char *) (m_large_memory->Ptr()); | ||||||
|  | 
 | ||||||
|  |                 const char *file_index_ptr = | ||||||
|  |                         data_layout->GetBlockPtr<char>(shared_memory, SharedDataLayout::FILE_INDEX_PATH); | ||||||
|  |                 file_index_path = boost::filesystem::path(file_index_ptr); | ||||||
|  |                 if (!boost::filesystem::exists(file_index_path)) { | ||||||
|  |                     SimpleLogger().Write(logDEBUG) << "Leaf file name " | ||||||
|  |                     << file_index_path.string(); | ||||||
|  |                     throw osrm::exception("Could not load leaf index file. " | ||||||
|  |                                                   "Is any data loaded into shared memory?"); | ||||||
|  |                 } | ||||||
|  | 
 | ||||||
|  |                 LoadGraph(); | ||||||
|  |                 LoadChecksum(); | ||||||
|  |                 LoadNodeAndEdgeInformation(); | ||||||
|  |                 LoadGeometries(); | ||||||
|  |                 LoadTimestamp(); | ||||||
|  |                 LoadViaNodeList(); | ||||||
|  |                 LoadNames(); | ||||||
|  |                 LoadCoreInformation(); | ||||||
|  | 
 | ||||||
|  |                 data_layout->PrintInformation(); | ||||||
|  | 
 | ||||||
|  |                 SimpleLogger().Write() << "number of geometries: " << m_coordinate_list->size(); | ||||||
|  |                 for (unsigned i = 0; i < m_coordinate_list->size(); ++i) | ||||||
|                 { |                 { | ||||||
|                     SimpleLogger().Write() << "coordinate " << i << " not valid"; |                     if (!GetCoordinateOfNode(i).is_valid()) | ||||||
|  |                     { | ||||||
|  |                         SimpleLogger().Write() << "coordinate " << i << " not valid"; | ||||||
|  |                     } | ||||||
|                 } |                 } | ||||||
|             } |             } | ||||||
|  |             SimpleLogger().Write(logDEBUG) << "Releasing exclusive lock"; | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
| 
 | 
 | ||||||
|  | |||||||
		Loading…
	
		Reference in New Issue
	
	Block a user