Create a session handle once in the transport ctor and reuse it for a…

…ll 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

@@ -11,10 +11,11 @@
 ### Bugs Fixed
 
 - Updated field type `CurlTransportOptions.Proxy` from `std::string` to `Azure::Nullable<std::string>`. 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)
 

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<RawResponse> 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

sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp

@@ -226,19 +226,19 @@ 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,
       WINHTTP_NO_PROXY_NAME,
       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<RawResponse> WinHttpTransport::Send(Request& request, Context co
 {
   auto handleManager = std::make_unique<_detail::HandleManager>(request, context);
 
-  CreateSessionHandle(handleManager);
   CreateConnectionHandle(handleManager);
   CreateRequestHandle(handleManager);