From 218784d82a2c440b655fbcc592ee3e75343ed15c Mon Sep 17 00:00:00 2001 From: Ahson Khan Date: Thu, 28 Apr 2022 21:19:55 +0000 Subject: [PATCH] Create a session handle once in the transport ctor and reuse it for all requests rather than creating a new one each time. (#3585) * Reuse the same session handle for all requests rather than creating a new one each time. * Move the session handle creation to the transport adapter ctor. * Update changelog entry. * Address PR feedback. * Change CreateSessionHandle to return a local session handle --- sdk/core/azure-core/CHANGELOG.md | 3 +- .../azure/core/http/win_http_transport.hpp | 32 +++++++++++-------- .../src/http/winhttp/win_http_transport.cpp | 31 ++++++++---------- 3 files changed, 34 insertions(+), 32 deletions(-) diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 1a4d9aca8..95428a500 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -11,10 +11,11 @@ ### Bugs Fixed - Updated field type `CurlTransportOptions.Proxy` from `std::string` to `Azure::Nullable`. This change allow to se an empty string to make libcurl ignore proxy settings from environment [](https://github.com/Azure/azure-sdk-for-cpp/issues/3537). +- [[#3548]](https://github.com/Azure/azure-sdk-for-cpp/issues/3548), [[#1098]](https://github.com/Azure/azure-sdk-for-cpp/issues/1098) Improve performance of the Http transport on Windows by reusing the same session handle across all requests. ### Other Changes -- [[#3581]](https://github.com/Azure/azure-sdk-for-cpp/issues/3581) Update log level in retry policy from warning to informational. +- [[#3581]](https://github.com/Azure/azure-sdk-for-cpp/issues/3581) Update log level in retry policy from warning to informational. ## 1.5.0 (2022-03-31) diff --git a/sdk/core/azure-core/inc/azure/core/http/win_http_transport.hpp b/sdk/core/azure-core/inc/azure/core/http/win_http_transport.hpp index 0e16fd84f..6090d6b49 100644 --- a/sdk/core/azure-core/inc/azure/core/http/win_http_transport.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/win_http_transport.hpp @@ -39,21 +39,19 @@ namespace Azure { namespace Core { namespace Http { { Context const& m_context; Request& m_request; - HINTERNET m_sessionHandle; HINTERNET m_connectionHandle; HINTERNET m_requestHandle; HandleManager(Request& request, Context const& context) : m_request(request), m_context(context) { - m_sessionHandle = NULL; m_connectionHandle = NULL; m_requestHandle = NULL; } ~HandleManager() { - // Close the handles and set them to null to avoid multiple calls to WinHTT to close the + // Close the handles and set them to null to avoid multiple calls to WinHTTP to close the // handles. if (m_requestHandle) { @@ -66,12 +64,6 @@ namespace Azure { namespace Core { namespace Http { WinHttpCloseHandle(m_connectionHandle); m_connectionHandle = NULL; } - - if (m_sessionHandle) - { - WinHttpCloseHandle(m_sessionHandle); - m_sessionHandle = NULL; - } } }; @@ -142,7 +134,11 @@ namespace Azure { namespace Core { namespace Http { private: WinHttpTransportOptions m_options; - void CreateSessionHandle(std::unique_ptr<_detail::HandleManager>& handleManager); + // This should remain immutable and not be modified after calling the ctor, to avoid threading + // issues. + HINTERNET m_sessionHandle = NULL; + + HINTERNET CreateSessionHandle(); void CreateConnectionHandle(std::unique_ptr<_detail::HandleManager>& handleManager); void CreateRequestHandle(std::unique_ptr<_detail::HandleManager>& handleManager); void Upload(std::unique_ptr<_detail::HandleManager>& handleManager); @@ -162,10 +158,7 @@ namespace Azure { namespace Core { namespace Http { * * @param options Optional parameter to override the default settings. */ - WinHttpTransport(WinHttpTransportOptions const& options = WinHttpTransportOptions()) - : m_options(options) - { - } + WinHttpTransport(WinHttpTransportOptions const& options = WinHttpTransportOptions()); /** * @brief Implements the HTTP transport interface to send an HTTP Request and produce an HTTP @@ -176,6 +169,17 @@ namespace Azure { namespace Core { namespace Http { * @return A unique pointer to an HTTP RawResponse. */ virtual std::unique_ptr Send(Request& request, Context const& context) override; + + ~WinHttpTransport() + { + // Close the handles and set them to null to avoid multiple calls to WinHTTP to close the + // handles. + if (m_sessionHandle) + { + WinHttpCloseHandle(m_sessionHandle); + m_sessionHandle = NULL; + } + } }; }}} // namespace Azure::Core::Http diff --git a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp index 3bdd40c83..3976ae1cc 100644 --- a/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp +++ b/sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp @@ -226,11 +226,11 @@ void GetErrorAndThrow(const std::string& exceptionMessage) throw Azure::Core::Http::TransportException(errorMessage); } -void WinHttpTransport::CreateSessionHandle(std::unique_ptr<_detail::HandleManager>& handleManager) +HINTERNET WinHttpTransport::CreateSessionHandle() { // Use WinHttpOpen to obtain a session handle. // The dwFlags is set to 0 - all WinHTTP functions are performed synchronously. - handleManager->m_sessionHandle = WinHttpOpen( + HINTERNET sessionHandle = WinHttpOpen( NULL, // Do not use a fallback user-agent string, and only rely on the header within the // request itself. WINHTTP_ACCESS_TYPE_NO_PROXY, @@ -238,7 +238,7 @@ void WinHttpTransport::CreateSessionHandle(std::unique_ptr<_detail::HandleManage WINHTTP_NO_PROXY_BYPASS, 0); - if (!handleManager->m_sessionHandle) + if (!sessionHandle) { // Errors include: // ERROR_WINHTTP_INTERNAL_ERROR @@ -253,31 +253,29 @@ void WinHttpTransport::CreateSessionHandle(std::unique_ptr<_detail::HandleManage #ifdef WINHTTP_OPTION_TCP_FAST_OPEN BOOL tcp_fast_open = TRUE; WinHttpSetOption( - handleManager->m_sessionHandle, - WINHTTP_OPTION_TCP_FAST_OPEN, - &tcp_fast_open, - sizeof(tcp_fast_open)); + sessionHandle, WINHTTP_OPTION_TCP_FAST_OPEN, &tcp_fast_open, sizeof(tcp_fast_open)); #endif #ifdef WINHTTP_OPTION_TLS_FALSE_START BOOL tls_false_start = TRUE; WinHttpSetOption( - handleManager->m_sessionHandle, - WINHTTP_OPTION_TLS_FALSE_START, - &tls_false_start, - sizeof(tls_false_start)); + sessionHandle, WINHTTP_OPTION_TLS_FALSE_START, &tls_false_start, sizeof(tls_false_start)); #endif // Enforce TLS version 1.2 auto tlsOption = WINHTTP_FLAG_SECURE_PROTOCOL_TLS1_2; if (!WinHttpSetOption( - handleManager->m_sessionHandle, - WINHTTP_OPTION_SECURE_PROTOCOLS, - &tlsOption, - sizeof(tlsOption))) + sessionHandle, WINHTTP_OPTION_SECURE_PROTOCOLS, &tlsOption, sizeof(tlsOption))) { GetErrorAndThrow("Error while enforcing TLS 1.2 for connection request."); } + + return sessionHandle; +} + +WinHttpTransport::WinHttpTransport(WinHttpTransportOptions const& options) + : m_options(options), m_sessionHandle(CreateSessionHandle()) +{ } void WinHttpTransport::CreateConnectionHandle( @@ -291,7 +289,7 @@ void WinHttpTransport::CreateConnectionHandle( // Specify an HTTP server. // This function always operates synchronously. handleManager->m_connectionHandle = WinHttpConnect( - handleManager->m_sessionHandle, + m_sessionHandle, StringToWideString(handleManager->m_request.GetUrl().GetHost()).c_str(), port == 0 ? INTERNET_DEFAULT_PORT : port, 0); @@ -656,7 +654,6 @@ std::unique_ptr WinHttpTransport::Send(Request& request, Context co { auto handleManager = std::make_unique<_detail::HandleManager>(request, context); - CreateSessionHandle(handleManager); CreateConnectionHandle(handleManager); CreateRequestHandle(handleManager);