Merge pull request #1902 from Project-OSRM/4.9.1

Bug fix release 4.9.1

data_structures/shared_memory_factory.hpp

@@ -99,6 +99,7 @@ class SharedMemory
 
     SharedMemory() = delete;
     SharedMemory(const SharedMemory &) = delete;
+    SharedMemory &operator=(const SharedMemory &) = delete;
 
     template <typename IdentifierT>
     SharedMemory(const boost::filesystem::path &lock_file,
@@ -208,11 +209,13 @@ class SharedMemory
 class SharedMemory
 {
     SharedMemory(const SharedMemory &) = delete;
+    SharedMemory &operator=(const SharedMemory &) = delete;
     // Remove shared memory on destruction
     class shm_remove
     {
       private:
         shm_remove(const shm_remove &) = delete;
+        shm_remove &operator=(const shm_remove &) = delete;
         char *m_shmid;
         bool m_initialized;
 
@@ -375,6 +378,7 @@ template <class LockFileT = OSRMLockFile> class SharedMemoryFactory_tmpl
 
     SharedMemoryFactory_tmpl() = delete;
     SharedMemoryFactory_tmpl(const SharedMemoryFactory_tmpl &) = delete;
+    SharedMemoryFactory_tmpl &operator=(const SharedMemoryFactory_tmpl &) = delete;
 };
 
 using SharedMemoryFactory = SharedMemoryFactory_tmpl<>;

descriptors/json_descriptor.hpp

@@ -306,6 +306,7 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
         round_about.leave_at_exit = 0;
         round_about.name_id = 0;
         std::string temp_dist, temp_length, temp_duration, temp_bearing, temp_instruction;
+        TravelMode last_travel_mode = TRAVEL_MODE_DEFAULT;
 
         // Fetch data from Factory and generate a string from it.
         for (const SegmentInformation &segment : description_factory.path_description)
@@ -354,6 +355,7 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
                         static_cast<unsigned>(round(post_turn_bearing_value)));
 
                     json_instruction_row.values.push_back(segment.travel_mode);
+                    last_travel_mode = segment.travel_mode;
 
                     // pre turn bearing
                     const double pre_turn_bearing_value = (segment.pre_turn_bearing / 10.);
@@ -389,6 +391,7 @@ template <class DataFacadeT> class JSONDescriptor final : public BaseDescriptor<
         json_last_instruction_row.values.push_back("0m");
         json_last_instruction_row.values.push_back(bearing::get(0.0));
         json_last_instruction_row.values.push_back(0.);
+        json_last_instruction_row.values.push_back(last_travel_mode);
         json_last_instruction_row.values.push_back(bearing::get(0.0));
         json_last_instruction_row.values.push_back(0.);
         json_instruction_array.values.push_back(json_last_instruction_row);

extractor/edge_based_graph_factory.cpp

@@ -445,10 +445,23 @@ void EdgeBasedGraphFactory::GenerateEdgeExpandedEdges(
                 }
                 else
                 {
-                    if ((node_u == node_w) && (m_node_based_graph->GetOutDegree(node_v) > 1))
+                    if (node_u == node_w && m_node_based_graph->GetOutDegree(node_v) > 1)
                     {
-                        ++skipped_uturns_counter;
+                        auto number_of_emmiting_bidirectional_edges = 0;
-                        continue;
+                        for (auto edge : m_node_based_graph->GetAdjacentEdgeRange(node_v))
+                        {
+                            auto target = m_node_based_graph->GetTarget(edge);
+                            auto reverse_edge = m_node_based_graph->FindEdge(target, node_v);
+                            if (!m_node_based_graph->GetEdgeData(reverse_edge).reversed)
+                            {
+                                ++number_of_emmiting_bidirectional_edges;
+                            }
+                        }
+                        if (number_of_emmiting_bidirectional_edges > 1)
+                        {
+                            ++skipped_uturns_counter;
+                            continue;
+                        }
                     }
                 }
 

library/osrm_impl.cpp

@@ -109,7 +109,16 @@ int OSRM::OSRM_impl::RunQuery(const RouteParameters &route_parameters, osrm::jso
     }
 
     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();
     return static_cast<int>(return_code);
 }

plugins/match.hpp

@@ -51,8 +51,6 @@ SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 template <class DataFacadeT> class MapMatchingPlugin : public BasePlugin
 {
-    constexpr static const unsigned max_number_of_candidates = 10;
-
     std::shared_ptr<SearchEngine<DataFacadeT>> search_engine_ptr;
 
     using ClassifierT = BayesClassifier<LaplaceDistribution, LaplaceDistribution, double>;
@@ -101,7 +99,9 @@ template <class DataFacadeT> class MapMatchingPlugin : public BasePlugin
     {
         osrm::matching::CandidateLists candidates_lists;
 
-        double query_radius = 10 * gps_precision;
+        // assuming the gps_precision is the standart-diviation of normal distribution that models
+        // GPS noise (in this model) this should give us the correct candidate with >0.95
+        double query_radius = 3 * gps_precision;
         double last_distance =
             coordinate_calculation::haversine_distance(input_coords[0], input_coords[1]);
 

plugins/trip.hpp

@@ -126,26 +126,18 @@ template <class DataFacadeT> class RoundTripPlugin final : public BasePlugin
         //          NodeID 3, 6, 7, 8    are in component 1
         //          => in_component = [0, 1, 2, 4, 5, 3, 6, 7, 8]
         //          => in_range = [0, 5]
-        SCC_Component(std::vector<NodeID> in_component, std::vector<size_t> in_range)
+        SCC_Component(std::vector<NodeID> in_component_nodes, std::vector<size_t> in_range)
-            : component(std::move(in_component)), range(std::move(in_range))
+            : component(std::move(in_component_nodes)), range(std::move(in_range))
         {
-            range.push_back(component.size());
-
-            BOOST_ASSERT_MSG(component.size() >= range.size(),
-                             "scc component and its ranges do not match");
             BOOST_ASSERT_MSG(component.size() > 0, "there's no scc component");
-            BOOST_ASSERT_MSG(*std::max_element(range.begin(), range.end()) <= component.size(),
+            BOOST_ASSERT_MSG(*std::max_element(range.begin(), range.end()) == component.size(),
                              "scc component ranges are out of bound");
-            BOOST_ASSERT_MSG(*std::min_element(range.begin(), range.end()) >= 0,
+            BOOST_ASSERT_MSG(*std::min_element(range.begin(), range.end()) == 0,
                              "invalid scc component range");
             BOOST_ASSERT_MSG(std::is_sorted(std::begin(range), std::end(range)),
                              "invalid component ranges");
         };
 
-        // constructor to use when whole graph is one single scc
-        SCC_Component(std::vector<NodeID> in_component)
-            : component(std::move(in_component)), range({0, component.size()}){};
-
         std::size_t GetNumberOfComponents() const
         {
             BOOST_ASSERT_MSG(range.size() > 0, "there's no range");
@@ -169,7 +161,8 @@ template <class DataFacadeT> class RoundTripPlugin final : public BasePlugin
             // whole graph is one scc
             std::vector<NodeID> location_ids(number_of_locations);
             std::iota(std::begin(location_ids), std::end(location_ids), 0);
-            return SCC_Component(std::move(location_ids));
+            std::vector<size_t> range = {0, location_ids.size()};
+            return SCC_Component(std::move(location_ids), std::move(range));
         }
 
         // Run TarjanSCC
@@ -194,6 +187,8 @@ template <class DataFacadeT> class RoundTripPlugin final : public BasePlugin
             range.push_back(prefix);
             prefix += scc.get_component_size(j);
         }
+        // senitel
+        range.push_back(components.size());
 
         for (std::size_t i = 0; i < number_of_locations; ++i)
         {
@@ -235,11 +230,13 @@ template <class DataFacadeT> class RoundTripPlugin final : public BasePlugin
         BOOST_ASSERT(min_route.segment_end_coordinates.size() == trip.size());
 
         std::vector<bool> uturns(trip.size() + 1);
+        BOOST_ASSERT(route_parameters.uturns.size() > 0);
         std::transform(trip.begin(), trip.end(), uturns.begin(),
                        [&route_parameters](const NodeID idx)
                        {
                            return route_parameters.uturns[idx];
                        });
+        BOOST_ASSERT(uturns.size() > 0);
         uturns.back() = route_parameters.uturns[trip.front()];
 
         search_engine_ptr->shortest_path(min_route.segment_end_coordinates, uturns, min_route);
@@ -316,13 +313,14 @@ template <class DataFacadeT> class RoundTripPlugin final : public BasePlugin
         {
             const auto component_size = scc.range[k + 1] - scc.range[k];
 
-            BOOST_ASSERT_MSG(component_size >= 0, "invalid component size");
+            BOOST_ASSERT_MSG(component_size > 0, "invalid component size");
+
+            std::vector<NodeID> scc_route;
+            NodeIDIterator start = std::begin(scc.component) + scc.range[k];
+            NodeIDIterator end = std::begin(scc.component) + scc.range[k + 1];
 
             if (component_size > 1)
             {
-                std::vector<NodeID> scc_route;
-                NodeIDIterator start = std::begin(scc.component) + scc.range[k];
-                NodeIDIterator end = std::begin(scc.component) + scc.range[k + 1];
 
                 if (component_size < BF_MAX_FEASABLE)
                 {
@@ -346,13 +344,13 @@ template <class DataFacadeT> class RoundTripPlugin final : public BasePlugin
                 //     return s;
                 // }();
 
-                route_result.push_back(std::move(scc_route));
             }
             else
             {
-                // if component only consists of one node, add it to the result routes
+                scc_route = std::vector<NodeID>(start, end);
-                route_result.emplace_back(scc.component[scc.range[k]]);
             }
+
+            route_result.push_back(std::move(scc_route));
         }
 
         // compute all round trip routes

plugins/viaroute.hpp

@@ -164,28 +164,9 @@ template <class DataFacadeT> class ViaRoutePlugin final : public BasePlugin
                                              route_parameters.uturns, raw_route);
         }
 
-        bool no_route = INVALID_EDGE_WEIGHT == raw_route.shortest_path_length;
-
-        std::unique_ptr<BaseDescriptor<DataFacadeT>> descriptor;
-        switch (descriptor_table.get_id(route_parameters.output_format))
-        {
-        case 1:
-            descriptor = osrm::make_unique<GPXDescriptor<DataFacadeT>>(facade);
-            break;
-        // case 2:
-        //      descriptor = osrm::make_unique<GEOJSONDescriptor<DataFacadeT>>();
-        //      break;
-        default:
-            descriptor = osrm::make_unique<JSONDescriptor<DataFacadeT>>(facade);
-            break;
-        }
-
-        descriptor->SetConfig(route_parameters);
-        descriptor->Run(raw_route, json_result);
-
         // we can only know this after the fact, different SCC ids still
         // allow for connection in one direction.
-        if (no_route)
+        if (raw_route.shortest_path_length == INVALID_EDGE_WEIGHT)
         {
             auto first_component_id = snapped_phantoms.front().component.id;
             auto not_in_same_component =
@@ -194,14 +175,36 @@ template <class DataFacadeT> class ViaRoutePlugin final : public BasePlugin
                             {
                                 return node.component.id != first_component_id;
                             });
+
             if (not_in_same_component)
             {
                 json_result.values["status_message"] = "Impossible route between points";
                 return Status::EmptyResult;
             }
+            else
+            {
+                json_result.values["status_message"] = "No route found between points";
+                return Status::Error;
+            }
         }
         else
         {
+            std::unique_ptr<BaseDescriptor<DataFacadeT>> descriptor;
+            switch (descriptor_table.get_id(route_parameters.output_format))
+            {
+            case 1:
+                descriptor = osrm::make_unique<GPXDescriptor<DataFacadeT>>(facade);
+                break;
+            // case 2:
+            //      descriptor = osrm::make_unique<GEOJSONDescriptor<DataFacadeT>>();
+            //      break;
+            default:
+                descriptor = osrm::make_unique<JSONDescriptor<DataFacadeT>>(facade);
+                break;
+            }
+
+            descriptor->SetConfig(route_parameters);
+            descriptor->Run(raw_route, json_result);
             json_result.values["status_message"] = "Found route between points";
         }
 

server/data_structures/shared_datafacade.hpp

@@ -239,10 +239,17 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
   public:
     virtual ~SharedDataFacade() {}
 
+    boost::shared_mutex data_mutex;
+
     SharedDataFacade()
     {
+        if (!SharedMemory::RegionExists(CURRENT_REGIONS))
+        {
+            throw osrm::exception("No shared memory blocks found, have you forgotten to run osrm-datastore?");
+        }
         data_timestamp_ptr = (SharedDataTimestamp *)SharedMemoryFactory::Get(
-                                 CURRENT_REGIONS, sizeof(SharedDataTimestamp), false, false)->Ptr();
+                                 CURRENT_REGIONS, sizeof(SharedDataTimestamp), false, false)
+                                 ->Ptr();
         CURRENT_LAYOUT = LAYOUT_NONE;
         CURRENT_DATA = DATA_NONE;
         CURRENT_TIMESTAMP = 0;
@@ -257,50 +264,71 @@ template <class EdgeDataT> class SharedDataFacade final : public BaseDataFacade<
             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 = 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";
         }
     }