From be7cf5f73d258a91cbc6da015b1ba796cd0fd39d Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 12 Jul 2019 17:22:12 -0700 Subject: [PATCH 1/3] Remove proxy settings detection behavior in "default proxy mode." This commit partially reverts https://github.com/microsoft/cpprestsdk/commit/eb108ada1ab23a46a09efb87f35c802e57e0832a in order to work around a reliability problem with WinHttpGetProxyForUrl. That function hangs unless there is an available thread pool thread to complete the WPAD request. As a result, if a customer issued ~512 concurrent HTTP requests, or otherwise needed that many thread pool threads, there would not be a thread available for WinHTTP to complete the operation, and the program would deadlock. Moreover this call to WinHttpGetDefaultProxyConfiguration is extremely expensive, taking ~20% of overall CPU for the entire program for some Azure Storage SDK customers. The function WinHttpGetProxyForUrlEx is supposed to help with this problem by being asynchronous, but that function was added in Windows 8, so we can't use it unconditionally. And on Windows 8.1 we already are using WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY instead of trying to do proxy autodetect ourselves. --- .../src/http/client/http_client_winhttp.cpp | 62 ++++++++++--------- 1 file changed, 32 insertions(+), 30 deletions(-) diff --git a/Release/src/http/client/http_client_winhttp.cpp b/Release/src/http/client/http_client_winhttp.cpp index a6e5a226b1..b482c8fd87 100644 --- a/Release/src/http/client/http_client_winhttp.cpp +++ b/Release/src/http/client/http_client_winhttp.cpp @@ -243,6 +243,24 @@ enum msg_body_type transfer_encoding_chunked }; +static DWORD WinHttpDefaultProxyConstant() CPPREST_NOEXCEPT +{ +#if _WIN32_WINNT >= _WIN32_WINNT_VISTA +#if _WIN32_WINNT < _WIN32_WINNT_WINBLUE + if (!IsWindows8Point1OrGreater()) + { + // Not Windows 8.1 or later, use the default proxy setting + return WINHTTP_ACCESS_TYPE_DEFAULT_PROXY; + } +#endif // _WIN32_WINNT < _WIN32_WINNT_WINBLUE + + // Windows 8.1 or later, use the automatic proxy setting + return WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY; +#else // ^^^ _WIN32_WINNT >= _WIN32_WINNT_VISTA ^^^ // vvv _WIN32_WINNT < _WIN32_WINNT_VISTA vvv + return WINHTTP_ACCESS_TYPE_DEFAULT_PROXY; +#endif // _WIN32_WINNT >= _WIN32_WINNT_VISTA +} + // Additional information necessary to track a WinHTTP request. class winhttp_request_context final : public request_context { @@ -818,38 +836,30 @@ class winhttp_client final : public _http_client_communicator ie_proxy_config proxyIE; DWORD access_type; - LPCWSTR proxy_name; + LPCWSTR proxy_name = WINHTTP_NO_PROXY_NAME; LPCWSTR proxy_bypass = WINHTTP_NO_PROXY_BYPASS; + m_proxy_auto_config = false; utility::string_t proxy_str; http::uri uri; const auto& config = client_config(); - - if (config.proxy().is_disabled()) + const auto& proxy = config.proxy(); + if (proxy.is_default()) + { + access_type = WinHttpDefaultProxyConstant(); + } + else if (proxy.is_disabled()) { access_type = WINHTTP_ACCESS_TYPE_NO_PROXY; - proxy_name = WINHTTP_NO_PROXY_NAME; } - else if (config.proxy().is_default() || config.proxy().is_auto_discovery()) + else if (proxy.is_auto_discovery()) { - // Use the default WinHTTP proxy by default. - access_type = WINHTTP_ACCESS_TYPE_DEFAULT_PROXY; - proxy_name = WINHTTP_NO_PROXY_NAME; - -#if _WIN32_WINNT < _WIN32_WINNT_VISTA - if (config.proxy().is_auto_discovery()) + access_type = WinHttpDefaultProxyConstant(); + if (access_type != WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY) { + // Windows 8 or earlier, do proxy autodetection ourselves m_proxy_auto_config = true; - } -#else // ^^^ _WIN32_WINNT < _WIN32_WINNT_VISTA ^^^ // vvv _WIN32_WINNT >= _WIN32_WINNT_VISTA vvv - if (IsWindows8Point1OrGreater()) - { - // Windows 8.1 and newer supports automatic proxy discovery and auto-fallback to IE proxy settings - access_type = WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY; - } - else - { - // However, if it is not configured... + proxy_info proxyDefault; if (!WinHttpGetDefaultProxyConfiguration(&proxyDefault) || proxyDefault.dwAccessType == WINHTTP_ACCESS_TYPE_NO_PROXY) @@ -881,13 +891,7 @@ class winhttp_client final : public _http_client_communicator } } } - - if (config.proxy().is_auto_discovery()) - { - m_proxy_auto_config = true; - } } -#endif // _WIN32_WINNT < _WIN32_WINNT_VISTA } else { @@ -1007,9 +1011,7 @@ class winhttp_client final : public _http_client_communicator if (m_proxy_auto_config) { - WINHTTP_AUTOPROXY_OPTIONS autoproxy_options; - memset(&autoproxy_options, 0, sizeof(WINHTTP_AUTOPROXY_OPTIONS)); - + WINHTTP_AUTOPROXY_OPTIONS autoproxy_options{}; if (m_proxy_auto_config_url.empty()) { autoproxy_options.dwFlags = WINHTTP_AUTOPROXY_AUTO_DETECT; From 629fa18d4b2af4dcd8b109d4d31187a58df98942 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 12 Jul 2019 18:20:42 -0700 Subject: [PATCH 2/3] clang-format --- Release/src/http/client/http_client_winhttp.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Release/src/http/client/http_client_winhttp.cpp b/Release/src/http/client/http_client_winhttp.cpp index b482c8fd87..249bfcd52c 100644 --- a/Release/src/http/client/http_client_winhttp.cpp +++ b/Release/src/http/client/http_client_winhttp.cpp @@ -256,7 +256,7 @@ static DWORD WinHttpDefaultProxyConstant() CPPREST_NOEXCEPT // Windows 8.1 or later, use the automatic proxy setting return WINHTTP_ACCESS_TYPE_AUTOMATIC_PROXY; -#else // ^^^ _WIN32_WINNT >= _WIN32_WINNT_VISTA ^^^ // vvv _WIN32_WINNT < _WIN32_WINNT_VISTA vvv +#else // ^^^ _WIN32_WINNT >= _WIN32_WINNT_VISTA ^^^ // vvv _WIN32_WINNT < _WIN32_WINNT_VISTA vvv return WINHTTP_ACCESS_TYPE_DEFAULT_PROXY; #endif // _WIN32_WINNT >= _WIN32_WINNT_VISTA } @@ -1011,7 +1011,7 @@ class winhttp_client final : public _http_client_communicator if (m_proxy_auto_config) { - WINHTTP_AUTOPROXY_OPTIONS autoproxy_options{}; + WINHTTP_AUTOPROXY_OPTIONS autoproxy_options {}; if (m_proxy_auto_config_url.empty()) { autoproxy_options.dwFlags = WINHTTP_AUTOPROXY_AUTO_DETECT; From 81fc600be87129629ce8a3354eb2f0300e5ea139 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Fri, 12 Jul 2019 22:51:05 -0700 Subject: [PATCH 3/3] Remove windows.h from web_utilities.h --- Release/include/cpprest/details/web_utilities.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/Release/include/cpprest/details/web_utilities.h b/Release/include/cpprest/details/web_utilities.h index aed7419c85..853d7614b1 100644 --- a/Release/include/cpprest/details/web_utilities.h +++ b/Release/include/cpprest/details/web_utilities.h @@ -10,10 +10,6 @@ ****/ #pragma once -#ifdef _WIN32 -#include -#endif // _WIN32 - #include "cpprest/asyncrt_utils.h" #include "cpprest/uri.h"