Merge branch 'master' into one_way_loop

CHANGELOG.md

@@ -18,10 +18,12 @@
       - 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 table result when source and destination on same one-way segment. [#5828](https://github.com/Project-OSRM/osrm-backend/pull/5828)
+      - 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)
       - CHANGED: updated extent of Hong Kong as left hand drive country. [#5535](https://github.com/Project-OSRM/osrm-backend/issues/5535)
+      - FIXED: corrected error message when failing to snap input coordinates [#5846](https://github.com/Project-OSRM/osrm-backend/pull/5846)
     - Infrastructure
       - REMOVED: STXXL support removed as STXXL became abandonware. [#5760](https://github.com/Project-OSRM/osrm-backend/pull/5760)
 # 5.21.0

include/engine/data_watchdog.hpp

@@ -56,12 +56,15 @@ class DataWatchdogImpl<AlgorithmT, datafacade::ContiguousInternalMemoryDataFacad
             static_region = *static_shared_region;
             updatable_region = *updatable_shared_region;
 
+            {
+                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,20 @@ class DataWatchdogImpl<AlgorithmT, datafacade::ContiguousInternalMemoryDataFacad
                         << (int)updatable_region.shm_key << " with timestamps "
                         << static_region.timestamp << " and " << updatable_region.timestamp;
 
+            {
+                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;

include/engine/plugins/plugin_base.hpp

@@ -371,6 +371,22 @@ class BasePlugin
         }
         return phantom_node_pairs;
     }
+
+    std::string MissingPhantomErrorMessage(const std::vector<PhantomNodePair> &phantom_nodes,
+                                           const std::vector<util::Coordinate> &coordinates) const
+    {
+        BOOST_ASSERT(phantom_nodes.size() < coordinates.size());
+        auto mismatch = std::mismatch(phantom_nodes.begin(),
+                                      phantom_nodes.end(),
+                                      coordinates.begin(),
+                                      coordinates.end(),
+                                      [](const auto &phantom_node, const auto &coordinate) {
+                                          return phantom_node.first.input_location == coordinate;
+                                      });
+        std::size_t missing_index = std::distance(phantom_nodes.begin(), mismatch.first);
+        return std::string("Could not find a matching segment for coordinate ") +
+               std::to_string(missing_index);
+    }
 };
 }
 }

src/engine/plugins/table.cpp

@@ -75,10 +75,8 @@ Status TablePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
 
     if (phantom_nodes.size() != params.coordinates.size())
     {
-        return Error("NoSegment",
+        return Error(
-                     std::string("Could not find a matching segment for coordinate ") +
+            "NoSegment", MissingPhantomErrorMessage(phantom_nodes, params.coordinates), result);
-                         std::to_string(phantom_nodes.size()),
-                     result);
     }
 
     auto snapped_phantoms = SnapPhantomNodes(phantom_nodes);

src/engine/plugins/trip.cpp

@@ -199,8 +199,7 @@ Status TripPlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithms,
     if (phantom_node_pairs.size() != number_of_locations)
     {
         return Error("NoSegment",
-                     std::string("Could not find a matching segment for coordinate ") +
+                     MissingPhantomErrorMessage(phantom_node_pairs, parameters.coordinates),
-                         std::to_string(phantom_node_pairs.size()),
                      result);
     }
     BOOST_ASSERT(phantom_node_pairs.size() == number_of_locations);

src/engine/plugins/viaroute.cpp

@@ -90,8 +90,7 @@ Status ViaRoutePlugin::HandleRequest(const RoutingAlgorithmsInterface &algorithm
     if (phantom_node_pairs.size() != route_parameters.coordinates.size())
     {
         return Error("NoSegment",
-                     std::string("Could not find a matching segment for coordinate ") +
+                     MissingPhantomErrorMessage(phantom_node_pairs, route_parameters.coordinates),
-                         std::to_string(phantom_node_pairs.size()),
                      result);
     }
     BOOST_ASSERT(phantom_node_pairs.size() == route_parameters.coordinates.size());

src/extractor/extractor.cpp

@@ -1,5 +1,6 @@
 #include "extractor/extractor.hpp"
 
+#include "extractor/compressed_edge_container.hpp"
 #include "extractor/compressed_node_based_graph_edge.hpp"
 #include "extractor/edge_based_edge.hpp"
 #include "extractor/extraction_containers.hpp"
@@ -11,47 +12,34 @@
 #include "extractor/maneuver_override_relation_parser.hpp"
 #include "extractor/name_table.hpp"
 #include "extractor/node_based_graph_factory.hpp"
-#include "extractor/raster_source.hpp"
 #include "extractor/restriction_filter.hpp"
+#include "extractor/restriction_index.hpp"
 #include "extractor/restriction_parser.hpp"
 #include "extractor/scripting_environment.hpp"
+#include "extractor/tarjan_scc.hpp"
+#include "extractor/way_restriction_map.hpp"
 
 #include "guidance/files.hpp"
 #include "guidance/guidance_processing.hpp"
 #include "guidance/segregated_intersection_classification.hpp"
 #include "guidance/turn_data_container.hpp"
 
-#include "storage/io.hpp"
-
 #include "util/exception.hpp"
 #include "util/exception_utils.hpp"
 #include "util/integer_range.hpp"
 #include "util/log.hpp"
-#include "util/range_table.hpp"
-#include "util/timing_util.hpp"
-
-#include "extractor/compressed_edge_container.hpp"
-#include "extractor/restriction_index.hpp"
-#include "extractor/way_restriction_map.hpp"
 #include "util/static_graph.hpp"
 #include "util/static_rtree.hpp"
+#include "util/timing_util.hpp"
 
 // Keep debug include to make sure the debug header is in sync with types.
 #include "util/debug.hpp"
 
-#include "extractor/tarjan_scc.hpp"
-
 #include <boost/assert.hpp>
-#include <boost/filesystem.hpp>
-#include <boost/filesystem/fstream.hpp>
-#include <boost/iterator/function_input_iterator.hpp>
-#include <boost/optional/optional.hpp>
-#include <boost/scope_exit.hpp>
 
 #include <osmium/handler/node_locations_for_ways.hpp>
 #include <osmium/index/map/flex_mem.hpp>
 #include <osmium/io/any_input.hpp>
-#include <osmium/osm/timestamp.hpp>
 #include <osmium/thread/pool.hpp>
 #include <osmium/visitor.hpp>
 
@@ -62,15 +50,11 @@
 #endif
 #include <tbb/pipeline.h>
 
-#include <cstdlib>
-
 #include <algorithm>
 #include <atomic>
 #include <bitset>
 #include <chrono>
-#include <future>
 #include <iostream>
-#include <iterator>
 #include <memory>
 #include <thread>
 #include <tuple>
@@ -193,7 +177,7 @@ std::vector<CompressedNodeBasedGraphEdge> toEdgeList(const util::NodeBasedDynami
 
     return edges;
 }
-}
+} // namespace
 
 /**
  * TODO: Refactor this function into smaller functions for better readability.
@@ -288,16 +272,6 @@ int Extractor::run(ScriptingEnvironment &scripting_environment)
     //
     // Luckily node based node ids still coincide with the coordinate array.
     // That's the reason we can only here write out the final compressed node based graph.
-
-    // Dumps to file asynchronously and makes sure we wait for its completion.
-    std::future<void> compressed_node_based_graph_writing;
-
-    BOOST_SCOPE_EXIT_ALL(&)
-    {
-        if (compressed_node_based_graph_writing.valid())
-            compressed_node_based_graph_writing.wait();
-    };
-
     files::writeCompressedNodeBasedGraph(config.GetPath(".osrm.cnbg").string(),
                                          toEdgeList(node_based_graph));
 
@@ -519,7 +493,6 @@ Extractor::ParseOSMData(ScriptingEnvironment &scripting_environment,
     // OSM elements Lua parser
     tbb::filter_t<SharedBuffer, ParsedBuffer> buffer_transformer(
         tbb::filter::parallel, [&](const SharedBuffer buffer) {
-
             ParsedBuffer parsed_buffer;
             parsed_buffer.buffer = buffer;
             scripting_environment.ProcessElements(*buffer,
@@ -540,7 +513,6 @@ Extractor::ParseOSMData(ScriptingEnvironment &scripting_environment,
     unsigned number_of_maneuver_overrides = 0;
     tbb::filter_t<ParsedBuffer, void> buffer_storage(
         tbb::filter::serial_in_order, [&](const ParsedBuffer &parsed_buffer) {
-
             number_of_nodes += parsed_buffer.resulting_nodes.size();
             // put parsed objects thru extractor callbacks
             for (const auto &result : parsed_buffer.resulting_nodes)
@@ -564,7 +536,6 @@ Extractor::ParseOSMData(ScriptingEnvironment &scripting_environment,
             {
                 extractor_callbacks->ProcessManeuverOverride(result);
             }
-
         });
 
     tbb::filter_t<SharedBuffer, std::shared_ptr<ExtractionRelationContainer>> buffer_relation_cache(
@@ -606,7 +577,6 @@ Extractor::ParseOSMData(ScriptingEnvironment &scripting_environment,
     tbb::filter_t<std::shared_ptr<ExtractionRelationContainer>, void> buffer_storage_relation(
         tbb::filter::serial_in_order,
         [&](const std::shared_ptr<ExtractionRelationContainer> parsed_relations) {
-
             number_of_relations += parsed_relations->GetRelationsNum();
             relations.Merge(std::move(*parsed_relations));
         });

unit_tests/library/table.cpp

@@ -242,6 +242,8 @@ BOOST_AUTO_TEST_CASE(test_table_no_segment_for_some_coordinates)
     BOOST_CHECK(rc == Status::Error);
     const auto code = json_result.values.at("code").get<json::String>().value;
     BOOST_CHECK_EQUAL(code, "NoSegment");
+    const auto message = json_result.values.at("message").get<json::String>().value;
+    BOOST_CHECK_EQUAL(message, "Could not find a matching segment for coordinate 0");
 }
 
 BOOST_AUTO_TEST_CASE(test_table_serialiaze_fb)