Upgrade Boost to 1.70, fix inefficient connection handling

A request to osrm-routed can be assigned to a thread which
is currently busy processing another request, even when there
are other threads/cores available. This unnecessarily delays
the response, and can make requests appear to hang when
awaiting CPU intensive requests to finish.

The issue looks like a bug in Boost.Asio multithreaded
networking stack.

osrm-routed server implementation is
heavily influenced by the HTTP server 3 example in the
Boost.Asio docs. By upgrading to Boost 1.70 and updating the
server connections to match the example provided in the 1.70
release, the problem is resolved.

The diff of the changes to the Boost.Asio stack are
vast, so it's difficult to identify the exact cause. However
the implementation change is to push the strand of execution
into the socket (and timer) objects, which suggests it could
fix the type of threading issue we are observing.

CMakeLists.txt

@@ -36,7 +36,7 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")
 
 if(ENABLE_MASON)
   # versions in use
-  set(MASON_BOOST_VERSION "1.65.1")
+  set(MASON_BOOST_VERSION "1.72.0")
   set(MASON_EXPAT_VERSION "2.2.0")
   set(MASON_LUA_VERSION "5.2.4")
   set(MASON_BZIP2_VERSION "1.0.6")
@@ -521,7 +521,7 @@ if(ENABLE_MASON)
   include_directories(SYSTEM ${CMAKE_CURRENT_SOURCE_DIR}/third_party/libosmium/include)
 else()
 
-  find_package(Boost 1.54 REQUIRED COMPONENTS ${BOOST_COMPONENTS})
+  find_package(Boost 1.70 REQUIRED COMPONENTS ${BOOST_COMPONENTS})
   add_dependency_includes(${Boost_INCLUDE_DIRS})
 
   find_package(TBB REQUIRED)
@@ -862,4 +862,4 @@ if (ENABLE_NODE_BINDINGS)
   endforeach()
   add_library(check-headers STATIC EXCLUDE_FROM_ALL ${sources})
   set_target_properties(check-headers PROPERTIES ARCHIVE_OUTPUT_DIRECTORY ${check_headers_dir})
-endif()
+endif()

docker/Dockerfile

@@ -1,14 +1,15 @@
-FROM debian:stretch-slim as builder
+FROM debian:buster-slim as builder
 ARG DOCKER_TAG
 ARG BUILD_CONCURRENCY
 RUN mkdir -p /src  && mkdir -p /opt
 COPY . /src
 WORKDIR /src
 
 RUN NPROC=${BUILD_CONCURRENCY:-$(grep -c ^processor /proc/cpuinfo 2>/dev/null || 1)} && \
+    echo 'deb http://deb.debian.org/debian buster-backports main' > /etc/apt/sources.list.d/backports.list && \
     apt-get update && \
     apt-get -y --no-install-recommends install cmake make git gcc g++ libbz2-dev libxml2-dev \
-    libzip-dev libboost1.62-all-dev lua5.2 liblua5.2-dev libtbb-dev -o APT::Install-Suggests=0 -o APT::Install-Recommends=0 && \
+    libzip-dev libboost1.71-all-dev lua5.2 liblua5.2-dev libtbb-dev -o APT::Install-Suggests=0 -o APT::Install-Recommends=0 && \
     echo "Building OSRM ${DOCKER_TAG}" && \
     git show --format="%H" | head -n1 > /opt/OSRM_GITSHA && \
     echo "Building OSRM gitsha $(cat /opt/OSRM_GITSHA)" && \
@@ -30,12 +31,14 @@ RUN NPROC=${BUILD_CONCURRENCY:-$(grep -c ^processor /proc/cpuinfo 2>/dev/null ||
 
 # Multistage build to reduce image size - https://docs.docker.com/engine/userguide/eng-image/multistage-build/#use-multi-stage-builds
 # Only the content below ends up in the image, this helps remove /src from the image (which is large)
-FROM debian:stretch-slim as runstage
+FROM debian:buster-slim as runstage
 RUN mkdir -p /src  && mkdir -p /opt
-RUN apt-get update && \
-    apt-get install -y --no-install-recommends libboost-program-options1.62.0 libboost-regex1.62.0 \
-        libboost-date-time1.62.0 libboost-chrono1.62.0 libboost-filesystem1.62.0 \
-        libboost-iostreams1.62.0 libboost-thread1.62.0 expat liblua5.2-0 libtbb2 &&\
+RUN echo 'deb http://deb.debian.org/debian buster-backports main' > /etc/apt/sources.list.d/backports.list && \
+    apt-get update && \
+    apt-get install -y --no-install-recommends libboost-program-options1.71.0 libboost-regex1.71.0 \
+        libboost-date-time1.71.0 libboost-chrono1.71.0 libboost-filesystem1.71.0 \
+        libboost-iostreams1.71.0 libboost-system1.71.0 libboost-thread1.71.0 \
+        expat liblua5.2-0 libtbb2 &&\
     rm -rf /var/lib/apt/lists/*
 COPY --from=builder /usr/local /usr/local
 COPY --from=builder /opt /opt

include/server/connection.hpp

@@ -37,7 +37,7 @@ class RequestHandler;
 class Connection : public std::enable_shared_from_this<Connection>
 {
   public:
-    explicit Connection(boost::asio::io_service &io_service, RequestHandler &handler);
+    explicit Connection(boost::asio::io_context &io_context, RequestHandler &handler);
     Connection(const Connection &) = delete;
     Connection &operator=(const Connection &) = delete;
 
@@ -60,7 +60,7 @@ class Connection : public std::enable_shared_from_this<Connection>
     std::vector<char> compress_buffers(const std::vector<char> &uncompressed_data,
                                        const http::compression_type compression_type);
 
-    boost::asio::io_service::strand strand;
+    boost::asio::strand<boost::asio::io_context::executor_type> strand;
     boost::asio::ip::tcp::socket TCP_socket;
     boost::asio::deadline_timer timer;
     RequestHandler &request_handler;

include/server/server.hpp

@@ -43,12 +43,12 @@ class Server
     }
 
     explicit Server(const std::string &address, const int port, const unsigned thread_pool_size)
-        : thread_pool_size(thread_pool_size), acceptor(io_service),
-          new_connection(std::make_shared<Connection>(io_service, request_handler))
+        : thread_pool_size(thread_pool_size), acceptor(io_context),
+          new_connection(std::make_shared<Connection>(io_context, request_handler))
     {
         const auto port_string = std::to_string(port);
 
-        boost::asio::ip::tcp::resolver resolver(io_service);
+        boost::asio::ip::tcp::resolver resolver(io_context);
         boost::asio::ip::tcp::resolver::query query(address, port_string);
         boost::asio::ip::tcp::endpoint endpoint = *resolver.resolve(query);
 
@@ -74,7 +74,7 @@ class Server
         for (unsigned i = 0; i < thread_pool_size; ++i)
         {
             std::shared_ptr<std::thread> thread = std::make_shared<std::thread>(
-                boost::bind(&boost::asio::io_service::run, &io_service));
+                boost::bind(&boost::asio::io_context::run, &io_context));
             threads.push_back(thread);
         }
         for (auto thread : threads)
@@ -83,7 +83,7 @@ class Server
         }
     }
 
-    void Stop() { io_service.stop(); }
+    void Stop() { io_context.stop(); }
 
     void RegisterServiceHandler(std::unique_ptr<ServiceHandlerInterface> service_handler_)
     {
@@ -96,15 +96,15 @@ class Server
         if (!e)
         {
             new_connection->start();
-            new_connection = std::make_shared<Connection>(io_service, request_handler);
+            new_connection = std::make_shared<Connection>(io_context, request_handler);
             acceptor.async_accept(
                 new_connection->socket(),
                 boost::bind(&Server::HandleAccept, this, boost::asio::placeholders::error));
         }
     }
 
     unsigned thread_pool_size;
-    boost::asio::io_service io_service;
+    boost::asio::io_context io_context;
     boost::asio::ip::tcp::acceptor acceptor;
     std::shared_ptr<Connection> new_connection;
     RequestHandler request_handler;

src/server/connection.cpp

@@ -17,8 +17,9 @@ namespace osrm
 namespace server
 {
 
-Connection::Connection(boost::asio::io_service &io_service, RequestHandler &handler)
-    : strand(io_service), TCP_socket(io_service), timer(io_service), request_handler(handler)
+Connection::Connection(boost::asio::io_context &io_context, RequestHandler &handler)
+    : strand(boost::asio::make_strand(io_context)), TCP_socket(strand), timer(strand),
+      request_handler(handler)
 {
 }
 
@@ -27,12 +28,11 @@ boost::asio::ip::tcp::socket &Connection::socket() { return TCP_socket; }
 /// Start the first asynchronous operation for the connection.
 void Connection::start()
 {
-    TCP_socket.async_read_some(
-        boost::asio::buffer(incoming_data_buffer),
-        strand.wrap(boost::bind(&Connection::handle_read,
-                                this->shared_from_this(),
-                                boost::asio::placeholders::error,
-                                boost::asio::placeholders::bytes_transferred)));
+    TCP_socket.async_read_some(boost::asio::buffer(incoming_data_buffer),
+                               boost::bind(&Connection::handle_read,
+                                           this->shared_from_this(),
+                                           boost::asio::placeholders::error,
+                                           boost::asio::placeholders::bytes_transferred));
 
     if (keep_alive)
     {
@@ -119,29 +119,28 @@ void Connection::handle_read(const boost::system::error_code &error, std::size_t
         // write result to stream
         boost::asio::async_write(TCP_socket,
                                  output_buffer,
-                                 strand.wrap(boost::bind(&Connection::handle_write,
-                                                         this->shared_from_this(),
-                                                         boost::asio::placeholders::error)));
+                                 boost::bind(&Connection::handle_write,
+                                             this->shared_from_this(),
+                                             boost::asio::placeholders::error));
     }
     else if (result == RequestParser::RequestStatus::invalid)
     { // request is not parseable
         current_reply = http::reply::stock_reply(http::reply::bad_request);
 
         boost::asio::async_write(TCP_socket,
                                  current_reply.to_buffers(),
-                                 strand.wrap(boost::bind(&Connection::handle_write,
-                                                         this->shared_from_this(),
-                                                         boost::asio::placeholders::error)));
+                                 boost::bind(&Connection::handle_write,
+                                             this->shared_from_this(),
+                                             boost::asio::placeholders::error));
     }
     else
     {
         // we don't have a result yet, so continue reading
-        TCP_socket.async_read_some(
-            boost::asio::buffer(incoming_data_buffer),
-            strand.wrap(boost::bind(&Connection::handle_read,
-                                    this->shared_from_this(),
-                                    boost::asio::placeholders::error,
-                                    boost::asio::placeholders::bytes_transferred)));
+        TCP_socket.async_read_some(boost::asio::buffer(incoming_data_buffer),
+                                   boost::bind(&Connection::handle_read,
+                                               this->shared_from_this(),
+                                               boost::asio::placeholders::error,
+                                               boost::asio::placeholders::bytes_transferred));
     }
 }