Skip to content

Commit

Permalink
Immediately close bad connections to prevent file exhaustion
Browse files Browse the repository at this point in the history
osrm-routed does not immediately clean up a keep-alive connection
when the client closes it. Instead it waits for five seconds
of inactivity before removing.

Given a setup with low file limits and clients opening and
closing a lot of keep-alive connections, it's possible for
osrm-routed to run out of file descriptors whilst it waits for
the clean-up to trigger.

Furthermore, this causes the connection acceptor loop to exit.
Even after the old connections are cleaned up, new ones
will not be created. Any new requests will block until the
server is restarted.

This commit improves the situation by:

- Immediately closing connections on error. This includes EOF errors
indicating that the client has closed the connection. This releases
resources early (including the open file) and doesn't wait for the
timer.

- Log when the acceptor loop exits. Whilst this means the behaviour
can still occur for reasons other than too many open files,
we will at least have visibility of the cause and can investigate further.
  • Loading branch information
mjjbell committed Aug 29, 2021
1 parent dca35dc commit 63ce4af
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 3 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
# Unreleased
- Changes from 5.25.0
- Misc:
- Fixed: Set osrm-routed to immediately close bad connections [#6112](https://github.com/Project-OSRM/osrm-backend/pull/6112)

# 5.25.0
- Changes from 5.24.0
Expand Down
4 changes: 4 additions & 0 deletions include/server/server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ class Server
new_connection->socket(),
boost::bind(&Server::HandleAccept, this, boost::asio::placeholders::error));
}
else
{
util::Log(logERROR) << "HandleAccept error: " << e.message();
}
}

unsigned thread_pool_size;
Expand Down
8 changes: 5 additions & 3 deletions src/server/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,10 @@
#include "server/request_parser.hpp"

#include <boost/algorithm/string/predicate.hpp>
#include <boost/assert.hpp>
#include <boost/bind.hpp>
#include <boost/iostreams/filter/gzip.hpp>
#include <boost/iostreams/filtering_stream.hpp>

#include <iterator>
#include <string>
#include <vector>

namespace osrm
Expand Down Expand Up @@ -48,6 +45,8 @@ void Connection::handle_read(const boost::system::error_code &error, std::size_t
{
if (error)
{
util::Log(logDEBUG) << "Connection read error: " << error.message();
handle_shutdown();
return;
}

Expand All @@ -73,6 +72,7 @@ void Connection::handle_read(const boost::system::error_code &error, std::size_t
current_request.endpoint = TCP_socket.remote_endpoint(ec).address();
if (ec)
{
util::Log(logDEBUG) << "Socket remote endpoint error: " << ec.message();
handle_shutdown();
return;
}
Expand Down Expand Up @@ -183,6 +183,8 @@ void Connection::handle_timeout(boost::system::error_code ec)

void Connection::handle_shutdown()
{
// Cancel timer to ensure all resources are released immediately on shutdown.
timer.cancel();
// Initiate graceful connection closure.
boost::system::error_code ignore_error;
TCP_socket.shutdown(boost::asio::ip::tcp::socket::shutdown_both, ignore_error);
Expand Down

0 comments on commit 63ce4af

Please sign in to comment.