Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix handling of timed out connections kept alive in connection pool under Unix #762

Merged
merged 5 commits into from
Aug 3, 2018
Merged

Fix handling of timed out connections kept alive in connection pool under Unix #762

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d87fa2b
Add asio_connection::was_closed_by_server() to reduce duplication
vadz May 8, 2018
fb08ce5
Fix checking for server-side closure of HTTPS connections
vadz May 8, 2018
f4f2348
Fix bug with using re-opened connections with ASIO
vadz May 8, 2018
6e8c021
Merge remote-tracking branch 'origin/master' into dev/roschuma/762
ras0219-msft Aug 3, 2018
d5b1b7a
Reduce duplicate code between ssl_proxy and asio_context in handle_re…
ras0219-msft Aug 3, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 74 additions & 52 deletions Release/src/http/client/http_client_asio.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
#endif
#include <boost/asio.hpp>
#include <boost/asio/ssl.hpp>
#include <boost/asio/ssl/error.hpp>
#include <boost/asio/steady_timer.hpp>
#include <boost/algorithm/string.hpp>
#include <boost/bind.hpp>
Expand Down Expand Up @@ -170,6 +171,45 @@ class asio_connection
bool keep_alive() const { return m_keep_alive; }
bool is_ssl() const { return m_ssl_stream ? true : false; }

// Check if the error code indicates that the connection was closed by the
// server: this is used to detect if a connection in the pool was closed during
// its period of inactivity and we should reopen it.
bool was_reused_and_closed_by_server(const boost::system::error_code& ec) const
{
if (!is_reused())
{
// Don't bother reopening the connection if it's a new one: in this
// case, even if the connection was really lost, it's still a real
// error and we shouldn't try to reopen it.
return false;
}

// These errors tell if connection was closed.
if ((boost::asio::error::eof == ec)
|| (boost::asio::error::connection_reset == ec)
|| (boost::asio::error::connection_aborted == ec))
{
return true;
}

if (is_ssl())
{
// For SSL connections, we can also get a different error due to
// incorrect secure connection shutdown if it was closed by the
// server due to inactivity. Unfortunately, the exact error we get
// in this case depends on the Boost.Asio version used.
#if BOOST_ASIO_VERSION >= 101008
if (boost::asio::ssl::error::stream_truncated == ec)
return true;
#else // Asio < 1.10.8 didn't have ssl::error::stream_truncated
if (boost::system::error_code(ERR_PACK(ERR_LIB_SSL, 0, SSL_R_SHORT_READ), boost::asio::error::get_ssl_category()) == ec)
return true;
#endif
}

return false;
}

template <typename Iterator, typename Handler>
void async_connect(const Iterator &begin, const Handler &handler)
{
Expand Down Expand Up @@ -578,37 +618,13 @@ class asio_context : public request_context, public std::enable_shared_from_this
return;
}

m_context->m_connection->upgrade_to_ssl(m_context->m_http_client->client_config().get_ssl_context_callback());
m_context->upgrade_to_ssl();

m_ssl_tunnel_established(m_context);
}
else
{
// These errors tell if connection was closed.
const bool socket_was_closed((boost::asio::error::eof == ec)
|| (boost::asio::error::connection_reset == ec)
|| (boost::asio::error::connection_aborted == ec));
if (socket_was_closed && m_context->m_connection->is_reused())
{
// Failed to write to socket because connection was already closed while it was in the pool.
// close() here ensures socket is closed in a robust way and prevents the connection from being put to the pool again.
m_context->m_connection->close();

// Create a new context and copy the request object, completion event and
// cancellation registration to maintain the old state.
// This also obtains a new connection from pool.
auto new_ctx = m_context->create_request_context(m_context->m_http_client, m_context->m_request);
new_ctx->m_request_completion = m_context->m_request_completion;
new_ctx->m_cancellationRegistration = m_context->m_cancellationRegistration;

auto client = std::static_pointer_cast<asio_client>(m_context->m_http_client);
// Resend the request using the new context.
client->send_request(new_ctx);
}
else
{
m_context->report_error("Failed to read HTTP status line from proxy", ec, httpclient_errorcode_context::readheader);
}
m_context->handle_failed_read_status_line(ec, "Failed to read HTTP status line from proxy");
}
}

Expand All @@ -619,7 +635,6 @@ class asio_context : public request_context, public std::enable_shared_from_this
boost::asio::streambuf m_response;
};


enum class http_proxy_type
{
none,
Expand Down Expand Up @@ -821,6 +836,11 @@ class asio_context : public request_context, public std::enable_shared_from_this
}

private:
void upgrade_to_ssl()
{
m_connection->upgrade_to_ssl(m_http_client->client_config().get_ssl_context_callback());
}

std::string generate_basic_auth_header()
{
std::string header;
Expand Down Expand Up @@ -1173,31 +1193,33 @@ class asio_context : public request_context, public std::enable_shared_from_this
}
else
{
// These errors tell if connection was closed.
const bool socket_was_closed((boost::asio::error::eof == ec)
|| (boost::asio::error::connection_reset == ec)
|| (boost::asio::error::connection_aborted == ec));
if (socket_was_closed && m_connection->is_reused())
{
// Failed to write to socket because connection was already closed while it was in the pool.
// close() here ensures socket is closed in a robust way and prevents the connection from being put to the pool again.
m_connection->close();

// Create a new context and copy the request object, completion event and
// cancellation registration to maintain the old state.
// This also obtains a new connection from pool.
auto new_ctx = create_request_context(m_http_client, m_request);
new_ctx->m_request_completion = m_request_completion;
new_ctx->m_cancellationRegistration = m_cancellationRegistration;

auto client = std::static_pointer_cast<asio_client>(m_http_client);
// Resend the request using the new context.
client->send_request(new_ctx);
}
else
{
report_error("Failed to read HTTP status line", ec, httpclient_errorcode_context::readheader);
}
handle_failed_read_status_line(ec, "Failed to read HTTP status line");
}
}

void handle_failed_read_status_line(const boost::system::error_code& ec, const char* generic_error_message)
{
if (m_connection->was_reused_and_closed_by_server(ec))
{
// Failed to write to socket because connection was already closed while it was in the pool.
// close() here ensures socket is closed in a robust way and prevents the connection from being put to the pool again.
m_connection->close();

// Create a new context and copy the request object, completion event and
// cancellation registration to maintain the old state.
// This also obtains a new connection from pool.
auto new_ctx = create_request_context(m_http_client, m_request);
new_ctx->m_request._get_impl()->instream().seek(0);
new_ctx->m_request_completion = m_request_completion;
new_ctx->m_cancellationRegistration = m_cancellationRegistration;

auto client = std::static_pointer_cast<asio_client>(m_http_client);
// Resend the request using the new context.
client->send_request(new_ctx);
}
else
{
report_error(generic_error_message, ec, httpclient_errorcode_context::readheader);
}
}

Expand Down