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.

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

Curl keep alive #5930

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 24 commits
Commits
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
43 changes: 43 additions & 0 deletions sdk/core/azure-core/inc/azure/core/http/curl_transport.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@
#include "azure/core/nullable.hpp"

#include <chrono>
#include <ctime>
#include <memory>
#include <string>
namespace Azure { namespace Core { namespace Test {
gearama marked this conversation as resolved.
Show resolved Hide resolved
class CurlTransport_VerifyKeepAliveHeaders_Test;
}}} // namespace Azure::Core::Test

namespace Azure { namespace Core { namespace Http {
class CurlNetworkConnection;
Expand All @@ -26,6 +30,32 @@ namespace Azure { namespace Core { namespace Http {
*
*/
constexpr std::chrono::milliseconds DefaultConnectionTimeout = std::chrono::minutes(5);

/**
* @brief The configuration options for the keep-alive feature.
*
* @remark The keep-alive feature allows the SDK to reuse the same connection to the service for
* multiple requests.
*/
class KeepAliveOptions {
public:
/**
* @brief The time in seconds that the host will allow an idle connection to remain open
* before it is closed.
*
* @remark A connection is idle if no data is sent or received by a host. A host
* may keep an idle connection open for longer than timeout seconds, but the host should
* attempt to retain a connection for at least timeout seconds.
*
*/
std::chrono::seconds ConnectionTimeout = std::chrono::seconds(0);

/**
* @brief The maximum number of requests that a host will allow over a single connection.
*
*/
std::size_t MaxRequests = size_t(0);
gearama marked this conversation as resolved.
Show resolved Hide resolved
};
} // namespace _detail

/**
Expand Down Expand Up @@ -151,6 +181,15 @@ namespace Azure { namespace Core { namespace Http {
*/
bool HttpKeepAlive = true;

/**
* @brief Options specified in the keep-alive request header
*
* @remark The keep-alive feature allows the SDK to reuse the same connection to the service for
* multiple requests. This field is populated if the Keep-Alive header is present in the
* request.
*/
Azure::Nullable<Azure::Core::Http::_detail::KeepAliveOptions> KeepAliveOptions;
gearama marked this conversation as resolved.
Show resolved Hide resolved

/**
* @brief This option determines whether libcurl verifies the authenticity of the peer's
* certificate.
Expand Down Expand Up @@ -200,6 +239,8 @@ namespace Azure { namespace Core { namespace Http {
* @brief Concrete implementation of an HTTP Transport that uses libcurl.
*/
class CurlTransport : public HttpTransport {
friend class Azure::Core::Test::CurlTransport_VerifyKeepAliveHeaders_Test;

private:
CurlTransportOptions m_options;

Expand All @@ -209,6 +250,8 @@ namespace Azure { namespace Core { namespace Http {
*/
virtual void OnUpgradedConnection(std::unique_ptr<CurlNetworkConnection>&&){};

void ValidateKeepAliveHeaders(Request& request, std::unique_ptr<RawResponse>& response);
gearama marked this conversation as resolved.
Show resolved Hide resolved
gearama marked this conversation as resolved.
Show resolved Hide resolved

public:
/**
* @brief Construct a new CurlTransport object.
Expand Down
128 changes: 122 additions & 6 deletions sdk/core/azure-core/src/http/curl/curl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -403,9 +403,44 @@ std::unique_ptr<RawResponse> CurlTransport::Send(Request& request, Context const
auto response = session->ExtractResponse();
// Move the ownership of the CurlSession (bodyStream) to the response
response->SetBodyStream(std::move(session));
// check the consistency of the keep alive headers between the request and the response
ValidateKeepAliveHeaders(request, response);

return response;
}

void CurlTransport::ValidateKeepAliveHeaders(
Request& request,
std::unique_ptr<RawResponse>& response)
{
// if the server supports keep alive the headers should be present in the response. If they are
// they should be the same as the request headers.
if (response->GetHeaders().find("Connection") != response->GetHeaders().end()
&& request.GetHeader("Connection").HasValue()
&& response->GetHeaders().find("Keep-Alive") != response->GetHeaders().end()
gearama marked this conversation as resolved.
Show resolved Hide resolved
&& request.GetHeader("Keep-Alive").HasValue()
// just in case the server sends the headers in a different case
&& Azure::Core::_internal::StringExtensions::ToLower(
response->GetHeaders().find("Connection")->second)
== Azure::Core::_internal::StringExtensions::ToLower(
gearama marked this conversation as resolved.
Show resolved Hide resolved
request.GetHeader("Connection").Value())
// just in case the server sends the keep-alive header in a different case
&& Azure::Core::_internal::StringExtensions::ToLower(
response->GetHeaders().find("Keep-Alive")->second)
== Azure::Core::_internal::StringExtensions::ToLower(
request.GetHeader("Keep-Alive").Value()))
{
Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Response has same keep-alive settings");
}
else
{
// cleanup keep-alive header in the request since they don't match up the response from
// the server.
request.RemoveHeader("Keep-Alive");
m_options.KeepAliveOptions.Reset();
}
}

CURLcode CurlSession::Perform(Context const& context)
{
// Set the session state
Expand Down Expand Up @@ -2220,10 +2255,18 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
{
g_curlConnectionPool.ConnectionPoolIndex.erase(hostPoolIndex);
}

Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Re-using connection from the pool.");
// return connection ref
return connection;
// if the connection is expired do not return it, let the code flow and return a new one.
if (!connection->IsKeepAliveExpired())
{
connection->IncreaseUsageCount();
Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Re-using connection from the pool.");
// return connection ref
return connection;
}
else
{
Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Connection expired. Discarding.");
}
}
}
}
Expand All @@ -2235,6 +2278,58 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
return std::make_unique<CurlConnection>(request, options, hostDisplayName, connectionKey);
}

Azure::Core::Http::_detail::KeepAliveOptions CurlConnection::ParseKeepAliveHeader(
std::string const& keepAlive)
{
// Parse the Keep-Alive header to determine if the connection should be kept alive.
// The Keep-Alive header is in the format of:
// Keep-Alive: timeout=5, max=1000
// The timeout is the number of seconds the connection is allowed to be idle before it is closed.
// The max is the maximum number of requests that can be made on the connection before it is
// closed.
// If the header is not present, the connection will be kept alive for the lifetime of the
// application.
Azure::Core::Http::_detail::KeepAliveOptions keepAliveOptions;
std::string const timeoutKey = "timeout=";
std::string const maxKey = "max=";
auto timeoutPos = keepAlive.find(timeoutKey);
auto maxPos = keepAlive.find(maxKey);
try
{

if (timeoutPos != std::string::npos)
{
auto timeoutEnd = keepAlive.find(',', timeoutPos);
if (timeoutEnd == std::string::npos)
{
timeoutEnd = keepAlive.size();
}

keepAliveOptions.ConnectionTimeout = std::chrono::seconds(std::stoi(keepAlive.substr(
gearama marked this conversation as resolved.
Show resolved Hide resolved
timeoutPos + timeoutKey.size(), timeoutEnd - timeoutPos - timeoutKey.size())));
}

if (maxPos != std::string::npos)
{
auto maxEnd = keepAlive.find(',', maxPos);
if (maxEnd == std::string::npos)
{
maxEnd = keepAlive.size();
}
keepAliveOptions.MaxRequests
= std::stoi(keepAlive.substr(maxPos + maxKey.size(), maxEnd - maxPos - maxKey.size()));
}
}
catch (std::invalid_argument const&)
{
Log::Write(
Logger::Level::Error,
"Failed to parse max value / timeout from Keep-Alive header: " + keepAlive);
return Azure::Core::Http::_detail::KeepAliveOptions();
}
return keepAliveOptions;
}

// Move the connection back to the connection pool. Push it to the front so it becomes the
// first connection to be picked next time some one ask for a connection to the pool (LIFO)
void CurlConnectionPool::MoveConnectionBackToPool(
Expand All @@ -2246,9 +2341,9 @@ void CurlConnectionPool::MoveConnectionBackToPool(
return; // The server has asked us to not re-use this connection.
}

if (connection->IsShutdown())
if (connection->IsShutdown() || connection->IsKeepAliveExpired())
{
// Can't re-used a shut down connection
// Can't re-used a shut down connection or an expired connection
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// Can't re-used a shut down connection or an expired connection
// Can't re-use a shutdown connection or an expired connection

return;
}

Expand Down Expand Up @@ -2309,8 +2404,29 @@ CurlConnection::CurlConnection(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
+ std::string("curl_easy_init returned Null"));
}

CURLcode result;

if (request.GetHeader("connection").HasValue() && request.GetHeader("keep-alive").HasValue())
{
auto connectionHeader = Azure::Core::_internal::StringExtensions::ToLower(
request.GetHeader("connection").Value());
gearama marked this conversation as resolved.
Show resolved Hide resolved
auto keepAliveHeader = Azure::Core::_internal::StringExtensions::ToLower(
request.GetHeader("keep-alive").Value());

if (connectionHeader == "keep-alive")
gearama marked this conversation as resolved.
Show resolved Hide resolved
{
auto keepAliveValue = ParseKeepAliveHeader(keepAliveHeader);
// if we have issues parsing this header , or the data in the header is invalid no point in
// setting it up
if (keepAliveValue.ConnectionTimeout != std::chrono::seconds(0)
|| keepAliveValue.MaxRequests > 0)
{
m_keepAliveOptions = keepAliveValue;
}
}
}

if (options.EnableCurlTracing)
{
if (!SetLibcurlOption(
Expand Down
72 changes: 70 additions & 2 deletions sdk/core/azure-core/src/http/curl/curl_connection_private.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@

/// From openssl/x509.h. Avoids needing to include openssl headers
typedef struct x509_store_ctx_st X509_STORE_CTX;

namespace Azure { namespace Core { namespace Test {
class CurlConnectionTest_ParseKeepAliveHeader_Test;
}}} // namespace Azure::Core::Test
namespace Azure { namespace Core {
namespace _detail {
/**
Expand Down Expand Up @@ -105,6 +107,12 @@ namespace Azure { namespace Core {
*/
virtual bool IsExpired() = 0;

/**
* @brief Checks whether this CURL connection is expired due to keep-alive settings.
*
*/
virtual bool IsKeepAliveExpired() { return false; };

/**
* @brief This function is used when working with streams to pull more data from the wire.
* Function will try to keep pulling data from socket until the buffer is all written or until
Expand Down Expand Up @@ -134,22 +142,39 @@ namespace Azure { namespace Core {
* @return `true` is the connection was shut it down; otherwise, `false`.
*/
bool IsShutdown() const { return m_isShutDown; }

/**
* @brief Increase the usage count.
*
*/
virtual void IncreaseUsageCount(){};

/**
* @brief Get the connection usage count.
*
* @return The usage count
*/
virtual size_t GetUsageCount() const { return 0; };
};

/**
* @brief CURL HTTP connection.
*
*/
class CurlConnection final : public CurlNetworkConnection {
friend class Azure::Core::Test::CurlConnectionTest_ParseKeepAliveHeader_Test;

private:
Azure::Core::_internal::UniqueHandle<CURL> m_handle;
curl_socket_t m_curlSocket;
std::chrono::steady_clock::time_point m_lastUseTime;
std::chrono::steady_clock::time_point m_firstUseTime = std::chrono::steady_clock::now();
std::string m_connectionKey;
// CRL validation is disabled by default to be consistent with WinHTTP behavior
bool m_enableCrlValidation{false};
// Allow the connection to proceed if retrieving the CRL failed.
bool m_allowFailedCrlRetrieval{true};
size_t m_usedCount = size_t(1);

static int CurlLoggingCallback(
CURL* handle,
Expand All @@ -161,6 +186,9 @@ namespace Azure { namespace Core {
static int CurlSslCtxCallback(CURL* curl, void* sslctx, void* parm);
int SslCtxCallback(CURL* curl, void* sslctx);
int VerifyCertificateError(int ok, X509_STORE_CTX* storeContext);
Azure::Nullable<Azure::Core::Http::_detail::KeepAliveOptions> m_keepAliveOptions;
Azure::Core::Http::_detail::KeepAliveOptions ParseKeepAliveHeader(
std::string const& keepAlive);

public:
/**
Expand Down Expand Up @@ -203,7 +231,34 @@ namespace Azure { namespace Core {
{
auto connectionOnWaitingTimeMs = std::chrono::duration_cast<std::chrono::milliseconds>(
std::chrono::steady_clock::now() - this->m_lastUseTime);
return connectionOnWaitingTimeMs.count() >= _detail::DefaultConnectionExpiredMilliseconds;
return connectionOnWaitingTimeMs.count() >= _detail::DefaultConnectionExpiredMilliseconds
|| IsKeepAliveExpired();
}

/**
* @brief Checks whether this CURL connection is expired due to keep-alive settings.
* @return `true` if this connection is considered expired; otherwise, `false`.
*/
bool IsKeepAliveExpired() override
{
// if we have keep alive options and we haven reached the max requests declare expired
if (m_keepAliveOptions.HasValue() && m_keepAliveOptions.Value().MaxRequests > 0
&& m_keepAliveOptions.Value().MaxRequests <= m_usedCount)
{
return true;
}

// if we have keep alive options and we have a connection timeout and the connection time
// frame has passed declare expired
if (m_keepAliveOptions.HasValue()
gearama marked this conversation as resolved.
Show resolved Hide resolved
&& m_keepAliveOptions.Value().ConnectionTimeout > std::chrono::seconds(0)
&& m_firstUseTime + m_keepAliveOptions.Value().ConnectionTimeout
< std::chrono::steady_clock::now())
{
return true;
}

return false;
}

/**
Expand Down Expand Up @@ -231,6 +286,19 @@ namespace Azure { namespace Core {
*/
CURLcode SendBuffer(uint8_t const* buffer, size_t bufferSize, Context const& context)
override;

/**
* @brief Increase the usage count.
*
*/
void IncreaseUsageCount() override { m_usedCount++; };

/**
* @brief Get the connection usage count.
*
* @return The usage count
*/
size_t GetUsageCount() const override { return m_usedCount; }
};
} // namespace Http
}} // namespace Azure::Core
2 changes: 1 addition & 1 deletion sdk/core/azure-core/test/ut/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED True)
if(BUILD_TRANSPORT_CURL)
SET(CURL_OPTIONS_TESTS curl_options_test.cpp)
SET(CURL_SESSION_TESTS curl_session_test_test.cpp curl_session_test.hpp)
SET(CURL_CONNECTION_POOL_TESTS curl_connection_pool_test.cpp)
SET(CURL_CONNECTION_POOL_TESTS curl_connection_pool_test.cpp curl_connection_tests.cpp)
endif()

if(RUN_LONG_UNIT_TESTS)
Expand Down
Loading