diff --git a/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems b/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems index 1d43809f4..aef605484 100644 --- a/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems +++ b/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems @@ -19,10 +19,6 @@ - - - - @@ -49,10 +45,6 @@ - - - - diff --git a/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems.filters b/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems.filters index 4b8f7bbcf..464602cef 100644 --- a/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems.filters +++ b/Build/libHttpClient.GDK.Shared/libHttpClient.GDK.Shared.vcxitems.filters @@ -19,9 +19,6 @@ {cebce833-dc13-4078-9dc3-23dec64d44e6} - - {184db77a-e1d6-458b-ae23-8e7cb1ac3238} - {70210c99-2f90-422b-b92b-53521fc41574} @@ -39,15 +36,6 @@ Source\Common\Win - - Source\HTTP\Curl - - - Source\HTTP\Curl - - - Source\HTTP\Curl - Source\Platform\GDK @@ -60,9 +48,6 @@ Source\Platform\Windows - - Source\HTTP\Curl - Source\WebSocket\Websocketpp @@ -72,24 +57,12 @@ Source\Common\Win - - Source\HTTP\Curl - - - Source\HTTP\Curl - - - Source\HTTP\Curl - Source\WebSocket\WinHttp Source\WebSocket\WinHttp - - Source\HTTP\Curl - Source\WebSocket\Websocketpp @@ -97,4 +70,4 @@ Source\WebSocket\Websocketpp - + \ No newline at end of file diff --git a/Build/libHttpClient.GDK.props b/Build/libHttpClient.GDK.props index 7ef22c71a..880b431e0 100644 --- a/Build/libHttpClient.GDK.props +++ b/Build/libHttpClient.GDK.props @@ -112,7 +112,6 @@ websocketpp; we don't use its serial-port transport, so disable it. --> __WRL_NO_DEFAULT_LIB__;_LIB;ASIO_DISABLE_SERIAL_PORT;$(libHttpClientDefine);%(PreprocessorDefinitions) - %(AdditionalIncludeDirectories);$(GDKCrossPlatformPath)GRDK\ExtensionLibraries\Xbox.XCurl.API\Include Guard /Zc:__cplusplus /ZH:SHA_256 /bigobj /Zi %(AdditionalOptions) HC_PLATFORM=HC_PLATFORM_GDK;HC_DATAMODEL=HC_DATAMODEL_LLP64;%(PreprocessorDefinitions) @@ -124,7 +123,7 @@ false true $(Console_Libs);%(AdditionalDependencies) - xgameruntime.lib;XCurl.lib;%(AdditionalDependencies) + xgameruntime.lib;%(AdditionalDependencies) xgameruntime.lib;%(AdditionalDependencies) diff --git a/Build/libHttpClient.GDK/libHttpClient.GDK.NoWebSockets.def b/Build/libHttpClient.GDK/libHttpClient.GDK.NoWebSockets.def index ba63f8d57..3af3b8fc9 100644 --- a/Build/libHttpClient.GDK/libHttpClient.GDK.NoWebSockets.def +++ b/Build/libHttpClient.GDK/libHttpClient.GDK.NoWebSockets.def @@ -97,4 +97,6 @@ EXPORTS HCTraceSetPlatformCallbacks HCTraceSetTraceToDebugger HCWinHttpResume - HCWinHttpSuspend \ No newline at end of file + HCWinHttpSuspend + HCSettingsSetGlobalRequestLimit + HCSettingsGetGlobalRequestLimit diff --git a/Build/libHttpClient.GDK/libHttpClient.GDK.def b/Build/libHttpClient.GDK/libHttpClient.GDK.def index 086ff7c1a..a728ec3f8 100644 --- a/Build/libHttpClient.GDK/libHttpClient.GDK.def +++ b/Build/libHttpClient.GDK/libHttpClient.GDK.def @@ -128,4 +128,5 @@ EXPORTS HCWebSocketSetPingInterval HCHttpCallRequestGetMaxReceiveBufferSize HCHttpCallRequestSetMaxReceiveBufferSize - + HCSettingsSetGlobalRequestLimit + HCSettingsGetGlobalRequestLimit diff --git a/Build/libHttpClient.Win32/libHttpClient.Win32.NoWebSockets.def b/Build/libHttpClient.Win32/libHttpClient.Win32.NoWebSockets.def index 3b3232907..dbd1cd548 100644 --- a/Build/libHttpClient.Win32/libHttpClient.Win32.NoWebSockets.def +++ b/Build/libHttpClient.Win32/libHttpClient.Win32.NoWebSockets.def @@ -117,4 +117,6 @@ EXPORTS HCHttpCallResponseSetGzipCompressed HCHttpCallRequestSetProgressReportFunction HCHttpCallRequestGetMaxReceiveBufferSize - HCHttpCallRequestSetMaxReceiveBufferSize \ No newline at end of file + HCHttpCallRequestSetMaxReceiveBufferSize + HCSettingsSetGlobalRequestLimit + HCSettingsGetGlobalRequestLimit diff --git a/Build/libHttpClient.Win32/libHttpClient.Win32.def b/Build/libHttpClient.Win32/libHttpClient.Win32.def index 46f16bc52..7eef255a5 100644 --- a/Build/libHttpClient.Win32/libHttpClient.Win32.def +++ b/Build/libHttpClient.Win32/libHttpClient.Win32.def @@ -149,3 +149,5 @@ EXPORTS HCWebSocketSetPingInterval HCHttpCallRequestGetMaxReceiveBufferSize HCHttpCallRequestSetMaxReceiveBufferSize + HCSettingsSetGlobalRequestLimit + HCSettingsGetGlobalRequestLimit diff --git a/Build/libHttpClient.import.props b/Build/libHttpClient.import.props index 53bb14d43..ff1b197be 100644 --- a/Build/libHttpClient.import.props +++ b/Build/libHttpClient.import.props @@ -65,13 +65,6 @@ - - - - - - - %(AdditionalLibraryDirectories);$(Console_SdkLibPath) diff --git a/Include/httpClient/httpClient.h b/Include/httpClient/httpClient.h index 72d696c1d..e61d32953 100644 --- a/Include/httpClient/httpClient.h +++ b/Include/httpClient/httpClient.h @@ -211,6 +211,44 @@ STDAPI_(void) HCRemoveCallRoutedHandler( /// If it is passed a null proxy, it will reset to default. Does not include proxying web socket traffic. STDAPI HCSetGlobalProxy(_In_z_ const char* proxyUri) noexcept; +/// +/// Sets the maximum number of HTTP requests allowed to be in flight at one time. +/// +/// The maximum number of concurrent HTTP requests. Passing 0 restores the device default. +/// Result code for this API operation. Possible values are S_OK, E_NOTIMPL, or E_FAIL. +/// +/// Requests submitted beyond this limit are queued and started automatically as earlier requests +/// complete, so HCHttpCallPerformAsync() never fails because of the limit. Callers may create and +/// submit as many HTTP calls as they like; only the number that reach the platform HTTP stack at +/// once is capped. +/// +/// The limit exists to bound the memory held by in-flight requests. It defaults to 12 on Xbox +/// consoles, where that budget is tightest, and is unlimited by default everywhere else, including +/// GDK on PC. The device is detected at runtime, so the same binary running on an Xbox console and +/// on PC will pick up different defaults. Titles that want a cap on PC should set one explicitly. +/// +/// This may be called before HCInitialize(). Changing the value does not affect requests that are +/// already in flight, and lowering it will not cancel them; the count drains naturally as they +/// complete. +/// +/// Platform support: admission control is implemented by the WinHTTP-based HTTP provider, so the +/// limit is only supported on GDK (Xbox and PC) and Win32. On UWP, Linux, Android, iOS and macOS +/// this returns E_NOTIMPL rather than storing a value that would never throttle anything. +/// +STDAPI HCSettingsSetGlobalRequestLimit(_In_ uint32_t limit) noexcept; + +/// +/// Gets the maximum number of HTTP requests allowed to be in flight at one time. +/// +/// Passes back the current concurrent HTTP request limit. 0xFFFFFFFF means unlimited. +/// Result code for this API operation. Possible values are S_OK, E_INVALIDARG, E_NOTIMPL, or E_FAIL. +/// +/// This may be called before HCInitialize(). When no limit has been set explicitly this passes back +/// the device default, which is 0xFFFFFFFF (unlimited) on every device except Xbox consoles. Returns +/// E_NOTIMPL on platforms that do not support the limit - see HCSettingsSetGlobalRequestLimit(). +/// +STDAPI HCSettingsGetGlobalRequestLimit(_Out_ uint32_t* limit) noexcept; + ///////////////////////////////////////////////////////////////////////////////////////// // Http APIs // diff --git a/Samples/Win32WebSocket/pch.cpp b/Samples/Win32WebSocket/pch.cpp index a38f8a37d..607a2953f 100644 Binary files a/Samples/Win32WebSocket/pch.cpp and b/Samples/Win32WebSocket/pch.cpp differ diff --git a/Source/Global/global.cpp b/Source/Global/global.cpp index e964ef12f..b89565002 100644 --- a/Source/Global/global.cpp +++ b/Source/Global/global.cpp @@ -8,6 +8,10 @@ #include "../Logger/trace_internal.h" #include "../Mock/lhc_mock.h" +#if HC_PLATFORM == HC_PLATFORM_GDK +#include "XSystem.h" +#endif + #ifndef HC_NOWEBSOCKETS #include "../WebSocket/hcwebsocket.h" #endif @@ -19,6 +23,46 @@ using namespace xbox::httpclient; NAMESPACE_XBOX_HTTP_CLIENT_BEGIN +// Unlimited is the historical behavior on every platform, so it stays the default everywhere +// except Xbox consoles. That keeps this change from regressing concurrency for titles that were +// never throttled before. +constexpr uint32_t c_unlimitedGlobalRequestLimit = UINT32_MAX; + +// 0 means "no explicit limit configured"; GetGlobalRequestLimit resolves that to the device default. +// Deliberately not part of http_singleton: this must be settable before HCInitialize. +static std::atomic g_globalRequestLimit{ 0 }; + +// Resolved per call rather than cached: the same Gaming.Desktop binary ships on Xbox consoles, GDK +// PC, Steam and Steam Deck, so the device is only knowable at runtime. Caching would also risk +// latching a wrong answer, because these APIs are callable before the game runtime is initialized. +static uint32_t DefaultGlobalRequestLimit() noexcept +{ +#if HC_PLATFORM == HC_PLATFORM_GDK + // Bounds the memory held by in-flight requests on Xbox consoles, where that budget is tightest. + // Declared here rather than at namespace scope so it is not an unused constant on the platforms + // that never consult it, which build with -Werror. + constexpr uint32_t c_consoleDefaultGlobalRequestLimit = 12; + + return XSystemGetDeviceType() == XSystemDeviceType::Pc + ? c_unlimitedGlobalRequestLimit + : c_consoleDefaultGlobalRequestLimit; +#else + return c_unlimitedGlobalRequestLimit; +#endif +} + +void SetGlobalRequestLimit(uint32_t limit) noexcept +{ + // Callers pass 0 to restore the default request limit. + g_globalRequestLimit.store(limit, std::memory_order_relaxed); +} + +uint32_t GetGlobalRequestLimit() noexcept +{ + uint32_t const limit = g_globalRequestLimit.load(std::memory_order_relaxed); + return limit == 0 ? DefaultGlobalRequestLimit() : limit; +} + HRESULT http_singleton::singleton_access( _In_ singleton_access_mode mode, _In_opt_ HCInitArgs* createArgs, diff --git a/Source/Global/global.h b/Source/Global/global.h index 54cfd1bd1..fc8ce210b 100644 --- a/Source/Global/global.h +++ b/Source/Global/global.h @@ -12,6 +12,14 @@ namespace log class logger; } +// Process-wide cap on the number of HTTP requests allowed in flight against the platform HTTP +// stack at once. Kept outside the http_singleton so it can be configured before HCInitialize. +// Enforcement lives in the platform HTTP provider; only the WinHTTP provider (GDK and Win32) +// implements admission control today, so on other platforms this value is stored and readable but +// does not throttle. +void SetGlobalRequestLimit(uint32_t limit) noexcept; +uint32_t GetGlobalRequestLimit() noexcept; + typedef struct http_retry_after_api_state { http_retry_after_api_state() = default; diff --git a/Source/Global/global_publics.cpp b/Source/Global/global_publics.cpp index 32e7a450e..3622bcd87 100644 --- a/Source/Global/global_publics.cpp +++ b/Source/Global/global_publics.cpp @@ -81,6 +81,43 @@ try } CATCH_RETURN() +STDAPI +HCSettingsSetGlobalRequestLimit(_In_ uint32_t limit) noexcept +try +{ +#if HC_PLATFORM == HC_PLATFORM_WIN32 || HC_PLATFORM == HC_PLATFORM_GDK + // Deliberately does not require initialization: the limit is process-wide state so it can be + // configured before HCInitialize creates the provider. + xbox::httpclient::SetGlobalRequestLimit(limit); + return S_OK; +#else + // Admission control lives in the WinHTTP provider, which is only built for Win32 and GDK. + // Failing here is deliberate: storing a value that can never throttle anything would let a + // title believe it had configured a cap that silently does nothing. + (void)limit; + return E_NOTIMPL; +#endif +} +CATCH_RETURN() + +STDAPI +HCSettingsGetGlobalRequestLimit(_Out_ uint32_t* limit) noexcept +try +{ +#if HC_PLATFORM == HC_PLATFORM_WIN32 || HC_PLATFORM == HC_PLATFORM_GDK + RETURN_HR_IF(E_INVALIDARG, !limit); + + *limit = xbox::httpclient::GetGlobalRequestLimit(); + return S_OK; +#else + // Symmetric with HCSettingsSetGlobalRequestLimit: there is no meaningful limit to report on + // platforms whose HTTP provider does not implement admission control. + (void)limit; + return E_NOTIMPL; +#endif +} +CATCH_RETURN() + STDAPI HCSetHttpCallPerformFunction( _In_ HCCallPerformFunction performFunc, diff --git a/Source/HTTP/Curl/CurlDynamicLoader.cpp b/Source/HTTP/Curl/CurlDynamicLoader.cpp deleted file mode 100644 index 1621e8eda..000000000 --- a/Source/HTTP/Curl/CurlDynamicLoader.cpp +++ /dev/null @@ -1,142 +0,0 @@ -#include "pch.h" -#include "CurlDynamicLoader.h" - -#if HC_PLATFORM == HC_PLATFORM_GDK - -#include -#include - -namespace xbox -{ -namespace httpclient -{ - -std::mutex CurlDynamicLoader::s_initMutex; -HC_UNIQUE_PTR CurlDynamicLoader::s_instance; - -CurlDynamicLoader& CurlDynamicLoader::GetInstance() -{ - std::lock_guard lock(s_initMutex); - if (!s_instance) - { - HC_TRACE_VERBOSE(HTTPCLIENT, "Creating CurlDynamicLoader instance"); - - // Use libHttpClient custom allocator hooks while staying within class access to private ctor - http_stl_allocator a{}; - s_instance = HC_UNIQUE_PTR{ new (a.allocate(1)) CurlDynamicLoader }; - } - return *s_instance; -} - -void CurlDynamicLoader::DestroyInstance() -{ - std::lock_guard lock(s_initMutex); - if (s_instance) - { - // Unique ptr with http_alloc_deleter ensures custom free hooks are used - s_instance.reset(); - } -} - -CurlDynamicLoader::~CurlDynamicLoader() -{ - Cleanup(); -} - -bool CurlDynamicLoader::Initialize() -{ - if (m_curlLibrary != nullptr) - { - HC_TRACE_VERBOSE(HTTPCLIENT, "XCurl.dll already loaded"); - return true; // Already loaded - } - - HC_TRACE_INFORMATION(HTTPCLIENT, "Attempting to load XCurl.dll"); - - // Try to load XCurl.dll - m_curlLibrary = LoadLibraryA("XCurl.dll"); - if (m_curlLibrary == nullptr) - { - DWORD error = GetLastError(); - HC_TRACE_ERROR(HTTPCLIENT, "Failed to load XCurl.dll. Error code: %lu", error); - return false; - } - - // Load all required functions - bool success = true; - - success &= LoadFunction(reinterpret_cast(curl_global_init_fn), "curl_global_init"); - success &= LoadFunction(reinterpret_cast(curl_global_cleanup_fn), "curl_global_cleanup"); - success &= LoadFunction(reinterpret_cast(curl_easy_init_fn), "curl_easy_init"); - success &= LoadFunction(reinterpret_cast(curl_easy_cleanup_fn), "curl_easy_cleanup"); - success &= LoadFunction(reinterpret_cast(curl_easy_setopt_fn), "curl_easy_setopt"); - success &= LoadFunction(reinterpret_cast(curl_easy_getinfo_fn), "curl_easy_getinfo"); - success &= LoadFunction(reinterpret_cast(curl_easy_strerror_fn), "curl_easy_strerror"); - success &= LoadFunction(reinterpret_cast(curl_slist_append_fn), "curl_slist_append"); - success &= LoadFunction(reinterpret_cast(curl_slist_free_all_fn), "curl_slist_free_all"); - success &= LoadFunction(reinterpret_cast(curl_multi_init_fn), "curl_multi_init"); - success &= LoadFunction(reinterpret_cast(curl_multi_cleanup_fn), "curl_multi_cleanup"); - success &= LoadFunction(reinterpret_cast(curl_multi_add_handle_fn), "curl_multi_add_handle"); - success &= LoadFunction(reinterpret_cast(curl_multi_remove_handle_fn), "curl_multi_remove_handle"); - success &= LoadFunction(reinterpret_cast(curl_multi_perform_fn), "curl_multi_perform"); - success &= LoadFunction(reinterpret_cast(curl_multi_info_read_fn), "curl_multi_info_read"); - - // Note: curl_multi_poll might not be available in older versions, so we make it optional - LoadFunction(reinterpret_cast(curl_multi_poll_fn), "curl_multi_poll"); - success &= LoadFunction(reinterpret_cast(curl_multi_wait_fn), "curl_multi_wait"); - - if (!success) - { - Cleanup(); - return false; - } - - HC_TRACE_INFORMATION(HTTPCLIENT, "XCurl.dll loaded successfully"); - return true; -} - -void CurlDynamicLoader::Cleanup() -{ - if (m_curlLibrary != nullptr) - { - HC_TRACE_INFORMATION(HTTPCLIENT, "Unloading XCurl.dll"); - FreeLibrary(m_curlLibrary); - m_curlLibrary = nullptr; - } - - // Reset all function pointers - curl_global_init_fn = nullptr; - curl_global_cleanup_fn = nullptr; - curl_easy_init_fn = nullptr; - curl_easy_cleanup_fn = nullptr; - curl_easy_setopt_fn = nullptr; - curl_easy_getinfo_fn = nullptr; - curl_easy_strerror_fn = nullptr; - curl_slist_append_fn = nullptr; - curl_slist_free_all_fn = nullptr; - curl_multi_init_fn = nullptr; - curl_multi_cleanup_fn = nullptr; - curl_multi_add_handle_fn = nullptr; - curl_multi_remove_handle_fn = nullptr; - curl_multi_perform_fn = nullptr; - curl_multi_info_read_fn = nullptr; - curl_multi_poll_fn = nullptr; - curl_multi_wait_fn = nullptr; -} - -bool CurlDynamicLoader::LoadFunction(FARPROC& funcPtr, const char* functionName) -{ - funcPtr = GetProcAddress(m_curlLibrary, functionName); - if (funcPtr == nullptr) - { - DWORD error = GetLastError(); - HC_TRACE_ERROR(HTTPCLIENT, "Failed to load function: %s. Error code: %lu", functionName, error); - return false; - } - return true; -} - -} // httpclient -} // xbox - -#endif // HC_PLATFORM == HC_PLATFORM_GDK diff --git a/Source/HTTP/Curl/CurlDynamicLoader.h b/Source/HTTP/Curl/CurlDynamicLoader.h deleted file mode 100644 index 821cede41..000000000 --- a/Source/HTTP/Curl/CurlDynamicLoader.h +++ /dev/null @@ -1,110 +0,0 @@ -#pragma once - -// -// This header is always includable across platforms. On non-GDK platforms, -// the macros are defined as direct calls and the dynamic loader class is absent. -// On GDK, the dynamic loader class is available and macros route through it. -// - -#if HC_PLATFORM == HC_PLATFORM_GDK - -#include -#include -#include -#include - -namespace xbox -{ -namespace httpclient -{ - -// Dynamic curl function pointers -class CurlDynamicLoader -{ -public: - // Initialization/Cleanup functions - using curl_global_init_ptr = CURLcode(*)(long flags); - using curl_global_cleanup_ptr = void(*)(); - - // Easy interface functions - using curl_easy_init_ptr = CURL*(*)(); - using curl_easy_cleanup_ptr = void(*)(CURL* curl); - using curl_easy_setopt_ptr = CURLcode(*)(CURL* curl, CURLoption option, ...); - using curl_easy_getinfo_ptr = CURLcode(*)(CURL* curl, CURLINFO info, ...); - using curl_easy_strerror_ptr = const char*(*)(CURLcode code); - - // String list functions - using curl_slist_append_ptr = struct curl_slist*(*)(struct curl_slist* list, const char* string); - using curl_slist_free_all_ptr = void(*)(struct curl_slist* list); - - // Multi interface functions - using curl_multi_init_ptr = CURLM*(*)(); - using curl_multi_cleanup_ptr = CURLMcode(*)(CURLM* multi_handle); - using curl_multi_add_handle_ptr = CURLMcode(*)(CURLM* multi_handle, CURL* curl_handle); - using curl_multi_remove_handle_ptr = CURLMcode(*)(CURLM* multi_handle, CURL* curl_handle); - using curl_multi_perform_ptr = CURLMcode(*)(CURLM* multi_handle, int* running_handles); - using curl_multi_info_read_ptr = CURLMsg*(*)(CURLM* multi_handle, int* msgs_in_queue); - using curl_multi_poll_ptr = CURLMcode(*)(CURLM* multi_handle, struct curl_waitfd extra_fds[], unsigned int extra_nfds, int timeout_ms, int* ret); - using curl_multi_wait_ptr = CURLMcode(*)(CURLM* multi_handle, struct curl_waitfd extra_fds[], unsigned int extra_nfds, int timeout_ms, int* numfds); - - // Function pointers - curl_global_init_ptr curl_global_init_fn = nullptr; - curl_global_cleanup_ptr curl_global_cleanup_fn = nullptr; - curl_easy_init_ptr curl_easy_init_fn = nullptr; - curl_easy_cleanup_ptr curl_easy_cleanup_fn = nullptr; - curl_easy_setopt_ptr curl_easy_setopt_fn = nullptr; - curl_easy_getinfo_ptr curl_easy_getinfo_fn = nullptr; - curl_easy_strerror_ptr curl_easy_strerror_fn = nullptr; - curl_slist_append_ptr curl_slist_append_fn = nullptr; - curl_slist_free_all_ptr curl_slist_free_all_fn = nullptr; - curl_multi_init_ptr curl_multi_init_fn = nullptr; - curl_multi_cleanup_ptr curl_multi_cleanup_fn = nullptr; - curl_multi_add_handle_ptr curl_multi_add_handle_fn = nullptr; - curl_multi_remove_handle_ptr curl_multi_remove_handle_fn = nullptr; - curl_multi_perform_ptr curl_multi_perform_fn = nullptr; - curl_multi_info_read_ptr curl_multi_info_read_fn = nullptr; - curl_multi_poll_ptr curl_multi_poll_fn = nullptr; - curl_multi_wait_ptr curl_multi_wait_fn = nullptr; - - static CurlDynamicLoader& GetInstance(); - // Frees the singleton instance and unloads XCurl.dll (via destructor -> Cleanup) - static void DestroyInstance(); - ~CurlDynamicLoader(); - - bool Initialize(); - void Cleanup(); - bool IsLoaded() const { return m_curlLibrary != nullptr; } - -private: - CurlDynamicLoader() = default; - - bool LoadFunction(FARPROC& funcPtr, const char* functionName); - - HMODULE m_curlLibrary = nullptr; - - // Thread safety - static std::mutex s_initMutex; - static HC_UNIQUE_PTR s_instance; -}; - -} // httpclient -} // xbox - -// GDK macro variants: route through dynamic loader and provide default returns when not loaded -#define CURL_CALL(func_name) ::xbox::httpclient::CurlDynamicLoader::GetInstance().func_name##_fn -#define CURL_INVOKE_OR(defaultRet, func, ...) \ - ((::xbox::httpclient::CurlDynamicLoader::GetInstance().IsLoaded()) ? \ - (::xbox::httpclient::CurlDynamicLoader::GetInstance().func##_fn(__VA_ARGS__)) : \ - (defaultRet)) -// Convenience when defaultRet == 0 (common for void-calls or zero-initialized return types) -#define CURL_INVOKE(func, ...) CURL_INVOKE_OR(0, func, __VA_ARGS__) - -#else // non-GDK - -// Non-GDK macro variants: call directly -#define CURL_CALL(func_name) func_name -#define CURL_INVOKE_OR(defaultRet, func, ...) func(__VA_ARGS__) -// Convenience when defaultRet == 0 -#define CURL_INVOKE(func, ...) func(__VA_ARGS__) - -#endif // HC_PLATFORM == HC_PLATFORM_GDK diff --git a/Source/HTTP/Curl/CurlEasyRequest.cpp b/Source/HTTP/Curl/CurlEasyRequest.cpp index ce8eec075..21e45ec94 100644 --- a/Source/HTTP/Curl/CurlEasyRequest.cpp +++ b/Source/HTTP/Curl/CurlEasyRequest.cpp @@ -18,21 +18,13 @@ CurlEasyRequest::CurlEasyRequest(CURL* curlEasyHandle, HCCallHandle hcCall, XAsy CurlEasyRequest::~CurlEasyRequest() { - (void)CURL_INVOKE(curl_easy_cleanup, m_curlEasyHandle); - (void)CURL_INVOKE(curl_slist_free_all, m_headers); + (void)curl_easy_cleanup(m_curlEasyHandle); + (void)curl_slist_free_all(m_headers); } Result> CurlEasyRequest::Initialize(HCCallHandle hcCall, XAsyncBlock* async) { -#if HC_PLATFORM == HC_PLATFORM_GDK - // Ensure curl is loaded - if (!CurlDynamicLoader::GetInstance().IsLoaded()) - { - HC_TRACE_ERROR(HTTPCLIENT, "CurlEasyRequest::Initialize: XCurl.dll not available"); - return E_HC_XCURL_REQUIRED; - } -#endif - CURL* curlEasyHandle{ CURL_CALL(curl_easy_init)() }; + CURL* curlEasyHandle{ curl_easy_init() }; if (!curlEasyHandle) { HC_TRACE_ERROR(HTTPCLIENT, "CurlEasyRequest::Initialize:: curl_easy_init failed"); @@ -48,8 +40,7 @@ Result> CurlEasyRequest::Initialize(HCCallHandle void* clientRequestBodyReadCallbackContext{}; RETURN_IF_FAILED(HCHttpCallRequestGetRequestBodyReadFunction(hcCall, &clientRequestBodyReadCallback, &bodySize, &clientRequestBodyReadCallbackContext)); -// Specify libcurl progress callback and create libcurl progress callback for non-GDK platforms since XCurl doesn't support libcurl progress callback -#if HC_PLATFORM != HC_PLATFORM_GDK +// Specify libcurl's progress callback if the caller asked for progress reporting. // Get LHC Progress callback functions size_t uploadMinimumProgressInterval; void* uploadProgressReportCallbackContext{}; @@ -68,32 +59,18 @@ Result> CurlEasyRequest::Initialize(HCCallHandle easyRequest->SetOpt(CURLOPT_XFERINFOFUNCTION, &ProgressReportCallback); easyRequest->SetOpt(CURLOPT_NOPROGRESS, 0L); } -#endif // we set both POSTFIELDSIZE and INFILESIZE because curl uses one or the // other depending on method // We are allowing Setops to happen with a bodySize of zero in linux to handle certain clients // not being able to handle handshakes without a fixed body size. - // The reason for an if def statement is to handle the behavioral differences in libCurl vs xCurl. - -#if HC_PLATFORM == HC_PLATFORM_GDK - if (bodySize > 0) - { - RETURN_IF_FAILED(easyRequest->SetOpt(CURLOPT_POSTFIELDSIZE, static_cast(bodySize))); - RETURN_IF_FAILED(easyRequest->SetOpt(CURLOPT_INFILESIZE, static_cast(bodySize))); - // read callback - RETURN_IF_FAILED(easyRequest->SetOpt(CURLOPT_READFUNCTION, &ReadCallback)); - RETURN_IF_FAILED(easyRequest->SetOpt(CURLOPT_READDATA, easyRequest.get())); - } -#else RETURN_IF_FAILED(easyRequest->SetOpt(CURLOPT_POSTFIELDSIZE, static_cast(bodySize))); RETURN_IF_FAILED(easyRequest->SetOpt(CURLOPT_INFILESIZE, static_cast(bodySize))); // read callback RETURN_IF_FAILED(easyRequest->SetOpt(CURLOPT_READFUNCTION, &ReadCallback)); RETURN_IF_FAILED(easyRequest->SetOpt(CURLOPT_READDATA, easyRequest.get())); -#endif // url & method char const* url = nullptr; @@ -183,7 +160,7 @@ void CurlEasyRequest::Complete(CURLcode result) HC_TRACE_INFORMATION(HTTPCLIENT, "CurlEasyRequest::m_errorBuffer='%s'", m_errorBuffer); long platformError = 0; - auto curle = CURL_CALL(curl_easy_getinfo)(m_curlEasyHandle, CURLINFO_OS_ERRNO, &platformError); + auto curle = curl_easy_getinfo(m_curlEasyHandle, CURLINFO_OS_ERRNO, &platformError); if (curle != CURLE_OK) { return Fail(HrFromCurle(curle)); @@ -192,13 +169,13 @@ void CurlEasyRequest::Complete(CURLcode result) HRESULT hr = HCHttpCallResponseSetNetworkErrorCode(m_hcCallHandle, E_FAIL, static_cast(platformError)); assert(SUCCEEDED(hr)); - hr = HCHttpCallResponseSetPlatformNetworkErrorMessage(m_hcCallHandle, CURL_CALL(curl_easy_strerror)(result)); + hr = HCHttpCallResponseSetPlatformNetworkErrorMessage(m_hcCallHandle, curl_easy_strerror(result)); assert(SUCCEEDED(hr)); } else { long httpStatus = 0; - auto curle = CURL_CALL(curl_easy_getinfo)(m_curlEasyHandle, CURLINFO_RESPONSE_CODE, &httpStatus); + auto curle = curl_easy_getinfo(m_curlEasyHandle, CURLINFO_RESPONSE_CODE, &httpStatus); if (curle != CURLE_OK) { return Fail(HrFromCurle(curle)); @@ -240,7 +217,7 @@ HRESULT CurlEasyRequest::AddHeader(char const* name, char const* value) noexcept } header.resize(static_cast(written)); - curl_slist* appended = CURL_CALL(curl_slist_append)(m_headers, header.c_str()); + curl_slist* appended = curl_slist_append(m_headers, header.c_str()); if (!appended) { m_headersBuffer.pop_back(); @@ -286,39 +263,6 @@ size_t CurlEasyRequest::ReadCallback(char* buffer, size_t size, size_t nitems, v request->m_requestBodyOffset += bytesWritten; -#if HC_PLATFORM == HC_PLATFORM_GDK - size_t uploadMinimumProgressInterval; - void* uploadProgressReportCallbackContext{}; - HCHttpCallProgressReportFunction uploadProgressReportFunction = nullptr; - hr = HCHttpCallRequestGetProgressReportFunction(request->m_hcCallHandle, true, &uploadProgressReportFunction, &uploadMinimumProgressInterval, &uploadProgressReportCallbackContext); - if (FAILED(hr)) - { - HC_TRACE_ERROR_HR(HTTPCLIENT, hr, "CurlEasyRequest::ReadCallback: failed getting Progress Report upload function"); - return 1; - } - - uint64_t dynamicBodySize{}; - uint64_t dynamicBodyBytesWritten{}; - HCHttpCallRequestGetDynamicBytesWritten(request->m_hcCallHandle, &dynamicBodySize, &dynamicBodyBytesWritten); - - uint64_t reportBytesWritten = request->m_requestBodyOffset; - uint64_t reportTotalBytes = bodySize; - if (dynamicBodySize > 0) - { - reportBytesWritten = dynamicBodyBytesWritten; - reportTotalBytes = dynamicBodySize; - } - - ReportProgress( - request->m_hcCallHandle, - uploadProgressReportFunction, - request->m_hcCallHandle->uploadMinimumProgressReportInterval, - reportBytesWritten, - reportTotalBytes, - uploadProgressReportCallbackContext, - &request->m_hcCallHandle->uploadLastProgressReport - ); -#endif return bytesWritten; } @@ -390,7 +334,7 @@ size_t CurlEasyRequest::WriteHeaderCallback(char* buffer, size_t size, size_t ni size_t CurlEasyRequest::GetResponseContentLength(CURL* curlHandle) { curl_off_t contentLength = 0; - CURL_CALL(curl_easy_getinfo)(curlHandle, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T, &contentLength); + curl_easy_getinfo(curlHandle, CURLINFO_CONTENT_LENGTH_DOWNLOAD_T, &contentLength); return contentLength; } @@ -436,39 +380,6 @@ size_t CurlEasyRequest::WriteDataCallback(char* buffer, size_t size, size_t nmem { request->m_responseBodyRemainingToRead -= bufferSize; -#if HC_PLATFORM == HC_PLATFORM_GDK - size_t downloadMinimumProgressInterval; - void* downloadProgressReportCallbackContext{}; - HCHttpCallProgressReportFunction downloadProgressReportFunction = nullptr; - hr = HCHttpCallRequestGetProgressReportFunction(request->m_hcCallHandle, false, &downloadProgressReportFunction, &downloadMinimumProgressInterval, &downloadProgressReportCallbackContext); - if (FAILED(hr)) - { - HC_TRACE_ERROR_HR(HTTPCLIENT, hr, "CurlEasyRequest::WriteDataCallback: failed getting Progress Report download function"); - return 1; - } - - uint64_t dynamicBodySize{}; - uint64_t dynamicBodyBytesWritten{}; - HCHttpCallResponseGetDynamicBytesWritten(request->m_hcCallHandle, &dynamicBodySize, &dynamicBodyBytesWritten); - - uint64_t reportBytesWritten = request->m_responseBodySize - request->m_responseBodyRemainingToRead; - uint64_t reportTotalBytes = request->m_responseBodySize; - if (dynamicBodySize > 0) - { - reportBytesWritten = dynamicBodyBytesWritten; - reportTotalBytes = dynamicBodySize; - } - - ReportProgress( - request->m_hcCallHandle, - downloadProgressReportFunction, - request->m_hcCallHandle->downloadMinimumProgressReportInterval, - reportBytesWritten, - reportTotalBytes, - downloadProgressReportCallbackContext, - &request->m_hcCallHandle->downloadLastProgressReport - ); -#endif } return bufferSize; diff --git a/Source/HTTP/Curl/CurlEasyRequest.h b/Source/HTTP/Curl/CurlEasyRequest.h index 1c1e1cb87..916ad8ee9 100644 --- a/Source/HTTP/Curl/CurlEasyRequest.h +++ b/Source/HTTP/Curl/CurlEasyRequest.h @@ -1,11 +1,5 @@ #pragma once -// Always include CurlDynamicLoader.h for macros and (on GDK) loader type -#include "CurlDynamicLoader.h" -// Http provider should work with other curl implementations as well. -// The logic in CurlMulti::Perform is optimized for XCurl, but should work on any curl implementation. -#if HC_PLATFORM != HC_PLATFORM_GDK #include -#endif #include "Result.h" namespace xbox @@ -76,7 +70,7 @@ class CurlEasyRequest template HRESULT CurlEasyRequest::SetOpt(CURLoption option, typename OptType::type v) noexcept { - CURLcode result = CURL_CALL(curl_easy_setopt)(m_curlEasyHandle, option, v); + CURLcode result = curl_easy_setopt(m_curlEasyHandle, option, v); if (result != CURLE_OK) { HC_TRACE_ERROR(HTTPCLIENT, "curl_easy_setopt(request, %d, value) failed with %d", option, result); diff --git a/Source/HTTP/Curl/CurlMulti.cpp b/Source/HTTP/Curl/CurlMulti.cpp index 8715e8e8a..bc9d91f71 100644 --- a/Source/HTTP/Curl/CurlMulti.cpp +++ b/Source/HTTP/Curl/CurlMulti.cpp @@ -1,17 +1,13 @@ #include "pch.h" #include "CurlMulti.h" -#include "CurlDynamicLoader.h" #include "CurlProvider.h" -#include -#include - namespace xbox { namespace httpclient { -// XCurl doesn't support curl_multi_timeout, so use a small, fixed delay between calls to curl_multi_perform +// Some curl implementations do not support curl_multi_timeout, so use a small, fixed delay between calls to curl_multi_perform #define PERFORM_DELAY_MS 50 #define POLL_TIMEOUT_MS 0 @@ -19,19 +15,10 @@ Result> CurlMulti::Initialize(XTaskQueuePortHandle work { assert(workPort); -#if HC_PLATFORM == HC_PLATFORM_GDK - // Ensure curl is loaded - if (!CurlDynamicLoader::GetInstance().IsLoaded()) - { - HC_TRACE_ERROR(HTTPCLIENT, "CurlMulti::Initialize: XCurl.dll not available"); - return E_HC_XCURL_REQUIRED; - } -#endif - http_stl_allocator a{}; HC_UNIQUE_PTR multi{ new (a.allocate(1)) CurlMulti }; - multi->m_curlMultiHandle = CURL_CALL(curl_multi_init)(); + multi->m_curlMultiHandle = curl_multi_init(); if (!multi->m_curlMultiHandle) { HC_TRACE_ERROR(HTTPCLIENT, "CurlMulti::Initialize: curl_multi_init failed"); @@ -52,13 +39,13 @@ CurlMulti::~CurlMulti() if (!m_easyRequests.empty()) { - HC_TRACE_WARNING(HTTPCLIENT, "CurlMulti::~XCurlMulti: Failing all active requests."); + HC_TRACE_WARNING(HTTPCLIENT, "CurlMulti::~CurlMulti: Failing all active requests."); FailAllRequests(E_UNEXPECTED); } if (m_curlMultiHandle) { - (void)CURL_INVOKE(curl_multi_cleanup, m_curlMultiHandle); + (void)curl_multi_cleanup(m_curlMultiHandle); } } @@ -72,7 +59,7 @@ HRESULT CurlMulti::AddRequest(HC_UNIQUE_PTR easyRequest) return E_FAIL; } - auto result = CURL_CALL(curl_multi_add_handle)(m_curlMultiHandle, easyRequest->Handle()); + auto result = curl_multi_add_handle(m_curlMultiHandle, easyRequest->Handle()); if (result != CURLM_OK) { HC_TRACE_ERROR(HTTPCLIENT, "CurlMulti::AddRequest: curl_multi_add_handle failed with CURLCode=%u", result); @@ -212,7 +199,7 @@ void CALLBACK CurlMulti::TaskQueueCallback(_In_opt_ void* context, _In_ bool can HRESULT CurlMulti::PerformStepLocked(int& runningRequests) noexcept { runningRequests = 0; - CURLMcode result = CURL_CALL(curl_multi_perform)(m_curlMultiHandle, &runningRequests); + CURLMcode result = curl_multi_perform(m_curlMultiHandle, &runningRequests); if (result != CURLM_OK) { HC_TRACE_ERROR(HTTPCLIENT, "CurlMulti::PerformStepLocked: curl_multi_perform failed with CURLMcode=%u", result); @@ -222,7 +209,7 @@ HRESULT CurlMulti::PerformStepLocked(int& runningRequests) noexcept int remainingMessages{ 1 }; // assume there is at least 1 message so loop is always entered while (remainingMessages) { - CURLMsg* message = CURL_CALL(curl_multi_info_read)(m_curlMultiHandle, &remainingMessages); + CURLMsg* message = curl_multi_info_read(m_curlMultiHandle, &remainingMessages); if (message) { switch (message->msg) @@ -232,7 +219,7 @@ HRESULT CurlMulti::PerformStepLocked(int& runningRequests) noexcept auto requestIter = m_easyRequests.find(message->easy_handle); assert(requestIter != m_easyRequests.end()); - result = CURL_CALL(curl_multi_remove_handle)(m_curlMultiHandle, message->easy_handle); + result = curl_multi_remove_handle(m_curlMultiHandle, message->easy_handle); if (result != CURLM_OK) { HC_TRACE_ERROR(HTTPCLIENT, "CurlMulti::PerformStepLocked: curl_multi_remove_handle failed with CURLMcode=%u", result); @@ -269,24 +256,12 @@ HRESULT CurlMulti::Perform() noexcept // Reschedule Perform if there are still running requests int workAvailable{ 0 }; CURLMcode result{ CURLM_OK }; -#if HC_PLATFORM == HC_PLATFORM_GDK - // Try curl_multi_poll first, fall back to curl_multi_wait if not available - if (CURL_CALL(curl_multi_poll)) - { - result = CURL_CALL(curl_multi_poll)(m_curlMultiHandle, nullptr, 0, POLL_TIMEOUT_MS, &workAvailable); - } - else - { - result = CURL_CALL(curl_multi_wait)(m_curlMultiHandle, nullptr, 0, POLL_TIMEOUT_MS, &workAvailable); - } -#elif defined(CURL_AT_LEAST_VERSION) && CURL_AT_LEAST_VERSION(7,69,0) - // On supported non-GDK platforms with libcurl >= 7.69.0, we can call curl_multi_poll directly. - static_assert(CURL_CALL(curl_multi_poll) == curl_multi_poll, "curl_multi_poll must be unconditionally available"); - result = CURL_CALL(curl_multi_poll)(m_curlMultiHandle, nullptr, 0, POLL_TIMEOUT_MS, &workAvailable); +#if defined(CURL_AT_LEAST_VERSION) && CURL_AT_LEAST_VERSION(7,69,0) + // With libcurl >= 7.69.0 we can call curl_multi_poll directly. + result = curl_multi_poll(m_curlMultiHandle, nullptr, 0, POLL_TIMEOUT_MS, &workAvailable); #else - // On supported non-GDK platforms with libcurl < 7.69.0, we must fall back to curl_multi_wait. - static_assert(CURL_CALL(curl_multi_wait) == curl_multi_wait, "curl_multi_wait must be unconditionally available"); - result = CURL_CALL(curl_multi_wait)(m_curlMultiHandle, nullptr, 0, POLL_TIMEOUT_MS, &workAvailable); + // With libcurl < 7.69.0 we must fall back to curl_multi_wait. + result = curl_multi_wait(m_curlMultiHandle, nullptr, 0, POLL_TIMEOUT_MS, &workAvailable); #endif UNREFERENCED_PARAMETER(result); @@ -297,61 +272,6 @@ HRESULT CurlMulti::Perform() noexcept return S_OK; } -size_t CurlMulti::ActiveRequestCount() noexcept -{ - std::lock_guard lock{ m_mutex }; - return m_easyRequests.size(); -} - -HRESULT CurlMulti::PerformUntilDrained(uint32_t timeoutMs) noexcept -{ - auto const deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(timeoutMs); - - for (;;) - { - int runningRequests{ 0 }; - size_t activeRequests{ 0 }; - - { - std::unique_lock lock{ m_mutex }; - if (m_easyRequests.empty()) - { - return S_OK; - } - - HRESULT hr = PerformStepLocked(runningRequests); - if (FAILED(hr)) - { - // Match the task-queue path: an unexpected CURLM error fails everything rather - // than leaving requests wedged while the title is suspending. - lock.unlock(); - HC_TRACE_ERROR_HR(HTTPCLIENT, hr, "CurlMulti::PerformUntilDrained: Perform failed. Failing all active requests."); - FailAllRequests(hr); - return hr; - } - - activeRequests = m_easyRequests.size(); - } - - if (activeRequests == 0) - { - return S_OK; - } - - if (std::chrono::steady_clock::now() >= deadline) - { - HC_TRACE_WARNING(HTTPCLIENT, "CurlMulti::PerformUntilDrained: timed out with %zu request(s) still active", activeRequests); - // __HRESULT_FROM_WIN32 (not HRESULT_FROM_WIN32) because this file also builds for - // Linux/Android/Apple, where pal.h supplies the double-underscore form and the - // ERROR_TIMEOUT constant but not the single-underscore macro. - return __HRESULT_FROM_WIN32(ERROR_TIMEOUT); - } - - // Mirrors the delay the task queue path uses between curl_multi_perform calls. - std::this_thread::sleep_for(std::chrono::milliseconds(PERFORM_DELAY_MS)); - } -} - void CurlMulti::FailAllRequests(HRESULT hr) noexcept { std::unique_lock lock{ m_mutex }; @@ -360,7 +280,7 @@ void CurlMulti::FailAllRequests(HRESULT hr) noexcept { for (auto& pair : m_easyRequests) { - auto result = CURL_INVOKE_OR(CURLM_OK, curl_multi_remove_handle, m_curlMultiHandle, pair.first); + auto result = curl_multi_remove_handle(m_curlMultiHandle, pair.first); if (FAILED(HrFromCurlm(result))) { HC_TRACE_ERROR(HTTPCLIENT, "CurlMulti::FailAllRequests: curl_multi_remove_handle failed with CURLCode=%u", result); diff --git a/Source/HTTP/Curl/CurlMulti.h b/Source/HTTP/Curl/CurlMulti.h index b727fc307..84fef32db 100644 --- a/Source/HTTP/Curl/CurlMulti.h +++ b/Source/HTTP/Curl/CurlMulti.h @@ -20,20 +20,6 @@ class CurlMulti // Wrapper around curl_multi_add_handle HRESULT AddRequest(HC_UNIQUE_PTR easyRequest); - // Drives curl_multi_perform on the *calling* thread until every active request has - // completed or timeoutMs elapses. - // - // The normal perform loop only advances when the caller-supplied task queue is being - // dispatched (see ScheduleTaskQueueCallback). During an app suspend a title is free to - // park its own queue, which would otherwise stall the loop and leave xCurl blocked in - // Curl_multi::WaitForActiveHandles until the suspend watchdog terminates the title - // (bug 63050439). The xCurl contract puts the "keep performing" duty on the multi - // consumer, so on suspend LHC drives the loop itself instead of relying on that queue. - HRESULT PerformUntilDrained(uint32_t timeoutMs) noexcept; - - // Number of requests still owned by this multi handle. - size_t ActiveRequestCount() noexcept; - // Asyncronously cleanup any outstanding requests static HRESULT CleanupAsync(HC_UNIQUE_PTR multi, XAsyncBlock* async); diff --git a/Source/HTTP/Curl/CurlProvider.cpp b/Source/HTTP/Curl/CurlProvider.cpp index 87abb6040..09beaf907 100644 --- a/Source/HTTP/Curl/CurlProvider.cpp +++ b/Source/HTTP/Curl/CurlProvider.cpp @@ -1,9 +1,6 @@ #include "pch.h" #include "CurlProvider.h" #include "CurlEasyRequest.h" -#include "CurlDynamicLoader.h" - -#include namespace xbox { @@ -25,9 +22,7 @@ HRESULT HrFromCurlm(CURLMcode c) noexcept switch (c) { case CURLMcode::CURLM_OK: return S_OK; -#if HC_PLATFORM == HC_PLATFORM_GDK - case CURLMcode::CURLM_BAD_FUNCTION_ARGUMENT: assert(false); return E_INVALIDARG; -#elif defined(CURL_AT_LEAST_VERSION) && CURL_AT_LEAST_VERSION(7,69,0) +#if defined(CURL_AT_LEAST_VERSION) && CURL_AT_LEAST_VERSION(7,69,0) case CURLMcode::CURLM_BAD_FUNCTION_ARGUMENT: assert(false); return E_INVALIDARG; #endif default: return E_FAIL; @@ -36,48 +31,12 @@ HRESULT HrFromCurlm(CURLMcode c) noexcept Result> CurlProvider::Initialize() { -#if HC_PLATFORM == HC_PLATFORM_GDK - // Initialize dynamic curl loader first - auto& loader = CurlDynamicLoader::GetInstance(); - if (!loader.Initialize()) - { - HC_TRACE_ERROR(HTTPCLIENT, "CurlProvider::Initialize: Failed to load XCurl.dll"); - // Ensure the loader is cleaned up if initialization fails - CurlDynamicLoader::DestroyInstance(); - return E_FAIL; - } - - CURLcode initRes = CURL_CALL(curl_global_init)(CURL_GLOBAL_ALL); - HRESULT initHr = HrFromCurle(initRes); - if (FAILED(initHr)) - { - // If curl init fails, unload XCurl and free the loader singleton - CurlDynamicLoader::DestroyInstance(); - return initHr; - } -#else - CURLcode initRes = CURL_CALL(curl_global_init)(CURL_GLOBAL_ALL); + CURLcode initRes = curl_global_init(CURL_GLOBAL_ALL); RETURN_IF_FAILED(HrFromCurle(initRes)); -#endif http_stl_allocator a{}; auto provider = HC_UNIQUE_PTR{ new (a.allocate(1)) CurlProvider }; -#if HC_PLATFORM == HC_PLATFORM_GDK - // Mirror WinHttpProvider: subscribe to PLM app state so the provider can keep the curl - // perform loop running while the title suspends. Without this the loop only advances when - // the title dispatches its own task queue, and a title that parks that queue on suspend - // leaves xCurl blocked in WaitForActiveHandles until the watchdog kills it (bug 63050439). - HRESULT registerHr = RegisterAppStateChangeNotification(CurlProvider::AppStateChangedCallback, provider.get(), &provider->m_appStateChangedToken); - if (FAILED(registerHr)) - { - // Suspend handling is a resilience feature; failing to subscribe must not stop HTTP from - // working, so log and continue rather than failing initialization. - HC_TRACE_ERROR_HR(HTTPCLIENT, registerHr, "CurlProvider::Initialize: RegisterAppStateChangeNotification failed; suspend handling disabled"); - provider->m_appStateChangedToken = nullptr; - } -#endif - return std::move(provider); } @@ -86,45 +45,19 @@ CurlProvider::~CurlProvider() // Either CleanupAsync was never called or CurlProvider shouldn't be destroyed until it completes. assert(!m_cleanupTasksRemaining); -#if HC_PLATFORM == HC_PLATFORM_GDK - if (m_appStateChangedToken) - { - UnregisterAppStateChangeNotification(m_appStateChangedToken); - m_appStateChangedToken = nullptr; - } -#endif - if (m_multiCleanupQueue) { XTaskQueueCloseHandle(m_multiCleanupQueue); } - // make sure XCurlMultis are cleaned up before curl_global_cleanup + // make sure CurlMultis are cleaned up before curl_global_cleanup m_curlMultis.clear(); -#if HC_PLATFORM == HC_PLATFORM_GDK - if (CurlDynamicLoader::GetInstance().IsLoaded()) - { - CURL_CALL(curl_global_cleanup)(); - } - // Free the dynamic loader singleton (unloads XCurl.dll via its destructor) - CurlDynamicLoader::DestroyInstance(); -#else - CURL_CALL(curl_global_cleanup)(); -#endif + curl_global_cleanup(); } HRESULT CurlProvider::PerformAsync(HCCallHandle hcCall, XAsyncBlock* async) noexcept { -#if HC_PLATFORM == HC_PLATFORM_GDK - // Check if curl is available before proceeding - if (!CurlDynamicLoader::GetInstance().IsLoaded()) - { - HC_TRACE_ERROR(HTTPCLIENT, "CurlProvider::PerformAsync: XCurl.dll not available"); - return E_HC_XCURL_REQUIRED; - } -#endif - XTaskQueuePortHandle workPort{ nullptr }; RETURN_IF_FAILED(XTaskQueueGetPort(async->queue, XTaskQueuePort::Work, &workPort)); @@ -250,87 +183,6 @@ void CALLBACK CurlProvider::MultiCleanupComplete(_Inout_ struct XAsyncBlock* asy } } -#if HC_PLATFORM == HC_PLATFORM_GDK - -// Total time the provider will spend driving curl_multi_perform while suspending. The xCurl -// contract requires the multi consumer to keep performing across suspend so xCurl can quiesce -// its handles; this bounds that work so a stuck request can never hold the suspend open longer -// than the platform's own watchdog budget. This is a budget for the WHOLE drain, shared across -// every CurlMulti, not a per-multi allowance -- a title with several task queues would otherwise -// multiply this by the number of multis and could still blow the suspend budget. -#define SUSPEND_DRAIN_TIMEOUT_MS 4000 - -void CurlProvider::Suspend() noexcept -{ - HC_TRACE_INFORMATION(HTTPCLIENT, "CurlProvider::Suspend"); - - // The lock is held for the whole drain so a concurrent CleanupAsync cannot move and destroy - // the CurlMultis while they are being performed. Completions are delivered through the - // async block's task queue rather than inline, so no request completion can re-enter - // PerformAsync on this thread while the lock is held. The drain is time-bounded, so the - // worst case for a blocked caller is SUSPEND_DRAIN_TIMEOUT_MS in total. - std::lock_guard lock{ m_mutex }; - - if (m_isSuspended) - { - return; - } - m_isSuspended = true; - - auto const deadline = std::chrono::steady_clock::now() + std::chrono::milliseconds(SUSPEND_DRAIN_TIMEOUT_MS); - - for (auto& pair : m_curlMultis) - { - CurlMulti* multi = pair.second.get(); - if (!multi || multi->ActiveRequestCount() == 0) - { - continue; - } - - // Give each multi only what is left of the shared budget so the total drain stays - // bounded regardless of how many multis (task queue work ports) exist. - auto const now = std::chrono::steady_clock::now(); - if (now >= deadline) - { - HC_TRACE_WARNING(HTTPCLIENT, "CurlProvider::Suspend: drain budget exhausted; remaining CurlMulti(s) not drained"); - break; - } - - auto const remainingMs = std::chrono::duration_cast(deadline - now).count(); - - HRESULT hr = multi->PerformUntilDrained(static_cast(remainingMs)); - if (FAILED(hr)) - { - HC_TRACE_WARNING_HR(HTTPCLIENT, hr, "CurlProvider::Suspend: CurlMulti did not fully drain before suspend"); - } - } -} - -void CurlProvider::Resume() noexcept -{ - HC_TRACE_INFORMATION(HTTPCLIENT, "CurlProvider::Resume"); - - std::lock_guard lock{ m_mutex }; - m_isSuspended = false; -} - -void CALLBACK CurlProvider::AppStateChangedCallback(BOOLEAN isSuspended, void* context) noexcept -{ - assert(context); - auto provider = static_cast(context); - - // RegisterAppStateChangeNotification reports "quiescing" as isSuspended == TRUE. - if (isSuspended) - { - provider->Suspend(); - } - else - { - provider->Resume(); - } -} - -#endif // HC_PLATFORM == HC_PLATFORM_GDK } // httpclient } // xbox diff --git a/Source/HTTP/Curl/CurlProvider.h b/Source/HTTP/Curl/CurlProvider.h index f919bb2a9..0f3267851 100644 --- a/Source/HTTP/Curl/CurlProvider.h +++ b/Source/HTTP/Curl/CurlProvider.h @@ -3,17 +3,8 @@ #include "Platform/IHttpProvider.h" #include "CurlMulti.h" #include "Result.h" -#if HC_PLATFORM == HC_PLATFORM_GDK -// When developing titles for Xbox consoles, you must use WinHTTP or xCurl. -// See https://docs.microsoft.com/en-us/gaming/gdk/_content/gc/networking/overviews/web-requests/http-networking for detail -#include -#include -#include "CurlDynamicLoader.h" -#else -// This http provider should work with other curl implementations as well. -// The logic in CurlMulti::Perform is optimized for XCurl, but should work on any curl implementation. +// This http provider is built for platforms that link libcurl directly. #include -#endif namespace xbox { @@ -39,28 +30,12 @@ struct CurlProvider : public IHttpProvider HRESULT CleanupAsync(XAsyncBlock* async) noexcept override; -#if HC_PLATFORM == HC_PLATFORM_GDK -public: // Suspend/resume handling (public so tests can drive it without real PLM transitions) - // Drives curl_multi_perform on this thread until outstanding requests drain, because the - // caller-supplied task queue that normally drives the perform loop may be parked while the - // title is suspended. See CurlMulti::PerformUntilDrained and bug 63050439. - void Suspend() noexcept; - void Resume() noexcept; -#endif - protected: CurlProvider() = default; static HRESULT CALLBACK CleanupAsyncProvider(XAsyncOp op, const XAsyncProviderData* data) noexcept; static void CALLBACK MultiCleanupComplete(_Inout_ struct XAsyncBlock* asyncBlock) noexcept; -#if HC_PLATFORM == HC_PLATFORM_GDK - static void CALLBACK AppStateChangedCallback(BOOLEAN isSuspended, void* context) noexcept; - - PAPPSTATE_REGISTRATION m_appStateChangedToken{ nullptr }; - bool m_isSuspended{ false }; -#endif - // Create an CurlMulti per work port http_internal_map> m_curlMultis{}; diff --git a/Source/HTTP/WinHttp/winhttp_connection.cpp b/Source/HTTP/WinHttp/winhttp_connection.cpp index 8cd3b4dae..1daa3aded 100644 --- a/Source/HTTP/WinHttp/winhttp_connection.cpp +++ b/Source/HTTP/WinHttp/winhttp_connection.cpp @@ -139,6 +139,17 @@ WinHttpConnection::~WinHttpConnection() } HCHttpCallCloseHandle(m_call); + + // Release this request's slot in the provider's global concurrency budget. Tied to object + // lifetime rather than complete_task because a connection can be torn down on paths that never + // complete the task (for example force-closed during suspend or provider shutdown); leaking a + // slot there would permanently shrink the budget until requests queued forever. + if (m_requestCompletedCallback) + { + auto callback = std::move(m_requestCompletedCallback); + m_requestCompletedCallback = nullptr; + callback(); + } } Result> WinHttpConnection::Initialize( @@ -1483,6 +1494,20 @@ void CALLBACK WinHttpConnection::completion_callback( { win32_cs_autolock cs{ &pRequestContext->m_lock }; pRequestContext->m_hRequest = nullptr; + + // Close the connect handle here rather than leaving it to the destructor. + // Callers that wait for this callback (PLM suspend) require every WinHTTP + // resource to be released before they destroy the session handles, and the + // destructor may not have run by then: m_connections holds only weak + // references, so the object's lifetime depends on refcounts that may still + // be held elsewhere. WinHttp guarantees no further callbacks for the + // request at this point, so the connect handle is safe to close. + if (pRequestContext->m_hConnection != nullptr) + { + WinHttpCloseHandle(pRequestContext->m_hConnection); + pRequestContext->m_hConnection = nullptr; + } + connectionClosedCallback = std::move(pRequestContext->m_connectionClosedCallback); pRequestContext->m_state = ConnectionState::Closed; } diff --git a/Source/HTTP/WinHttp/winhttp_connection.h b/Source/HTTP/WinHttp/winhttp_connection.h index 444e8600c..e95b80362 100644 --- a/Source/HTTP/WinHttp/winhttp_connection.h +++ b/Source/HTTP/WinHttp/winhttp_connection.h @@ -180,6 +180,10 @@ class WinHttpConnection : public std::enable_shared_from_this // Called by WinHttpProvider to force close on PLM Suspend or shutdown HRESULT Close(ConnectionClosedCallback callback); + // Invoked exactly once when the HTTP request completes, so WinHttpProvider can release the + // request's slot in the global concurrency budget and promote a queued request. Not used for + // WebSocket connections, which are long-lived and do not consume budget slots. + void SetRequestCompletedCallback(std::function callback) { m_requestCompletedCallback = std::move(callback); } private: WinHttpConnection( HINTERNET hSession, @@ -290,6 +294,7 @@ class WinHttpConnection : public std::enable_shared_from_this HCCallHandle m_call; // ref-counted, released in destructor Uri m_uri; XAsyncBlock* m_asyncBlock = nullptr; // non-owning + std::function m_requestCompletedCallback; XPlatSecurityInformation const m_securityInformation{}; ConnectionClosedCallback m_connectionClosedCallback; diff --git a/Source/HTTP/WinHttp/winhttp_provider.cpp b/Source/HTTP/WinHttp/winhttp_provider.cpp index 4c6c6ff8a..fdfa3ca7e 100644 --- a/Source/HTTP/WinHttp/winhttp_provider.cpp +++ b/Source/HTTP/WinHttp/winhttp_provider.cpp @@ -1,5 +1,7 @@ #include "pch.h" #include "HTTP/httpcall.h" +#include "Global/global.h" +#include #include "winhttp_provider.h" #include "winhttp_connection.h" #include "uri.h" @@ -10,6 +12,20 @@ NAMESPACE_XBOX_HTTP_CLIENT_BEGIN +struct WinHttpProvider::CloseContext +{ + ~CloseContext() + { + if (connectionsClosedEvent) + { + CloseHandle(connectionsClosedEvent); + } + } + + HANDLE connectionsClosedEvent{ nullptr }; + std::atomic openConnections{ 0 }; +}; + Result> WinHttpProvider::Initialize() { http_stl_allocator a{}; @@ -18,18 +34,17 @@ Result> WinHttpProvider::Initialize() RETURN_IF_FAILED(XTaskQueueCreate(XTaskQueueDispatchMode::Immediate, XTaskQueueDispatchMode::Immediate, &provider->m_immediateQueue)); #if HC_PLATFORM == HC_PLATFORM_GDK - if (XGameRuntimeIsFeatureAvailable(XGameRuntimeFeature::XNetworking)) - { - RETURN_IF_FAILED(XNetworkingRegisterConnectivityHintChanged(provider->m_immediateQueue, provider.get(), WinHttpProvider::NetworkConnectivityChangedCallback, &provider->m_networkConnectivityChangedToken)); - } - else + // Soft-fail: PLM suspend handling is a resilience feature, so losing it must not cost the + // title HTTP entirely. RegisterAppStateChangeNotification resolves from + // api-ms-win-core-psm-appnotify, which is not guaranteed to be present in every configuration + // the GDK runs in. Log and continue without suspend notifications; the destructor tolerates a + // null token, and Suspend/Resume simply never fire. + HRESULT registerHr = RegisterAppStateChangeNotification(WinHttpProvider::AppStateChangedCallback, provider.get(), &provider->m_appStateChangedToken); + if (FAILED(registerHr)) { - // XNetworking not available (e.g., PC GDK build), assume network is ready - provider->m_networkInitialized = true; + HC_TRACE_ERROR_HR(HTTPCLIENT, registerHr, "WinHttpProvider::Initialize: RegisterAppStateChangeNotification failed; suspend handling disabled"); + provider->m_appStateChangedToken = nullptr; } - - RETURN_IF_FAILED(RegisterAppStateChangeNotification(WinHttpProvider::AppStateChangedCallback, provider.get(), &provider->m_appStateChangedToken)); - #endif // HC_PLATFORM == HC_PLATFORM_GDK return std::move(provider); @@ -48,29 +63,36 @@ WinHttpProvider::~WinHttpProvider() UnregisterAppStateChangeNotification(m_appStateChangedToken); } - if (XGameRuntimeIsFeatureAvailable(XGameRuntimeFeature::XNetworking)) - { - if (m_networkConnectivityChangedToken.token) - { - XNetworkingUnregisterConnectivityHintChanged(m_networkConnectivityChangedToken, true); - } - } + // Unregistering above stops new PLM notifications, but one may already be executing. Take the + // suspend lock so teardown cannot run concurrently with an in-progress Suspend or Resume. + std::unique_lock suspendLock{ m_suspendLock }; #endif - HRESULT hr = CloseAllConnections(); + // INFINITE on the shutdown path: waiting for every connection to report closed is what keeps a + // WinHTTP callback from running after this object, the singleton, and any caller-supplied + // memory hooks are gone. + HRESULT hr = CloseAllConnections(INFINITE); if (FAILED(hr)) { HC_TRACE_ERROR_HR(HTTPCLIENT, hr, "WinHttpProvider::CloseAllConnections failed during shutdown"); } - for (auto& pair : m_hSessions) { - if (pair.second) + std::lock_guard lock{ m_lock }; + for (auto& pair : m_hSessions) { - WinHttpCloseHandle(pair.second); + if (pair.second) + { + WinHttpCloseHandle(pair.second); + } } + m_hSessions.clear(); } - m_hSessions.clear(); + +#if HC_PLATFORM == HC_PLATFORM_GDK + // Release before the mutex itself is destroyed with the object. + suspendLock.unlock(); +#endif } WinHttpWebSocketExports GetWinHttpWebSocketExportsHelper() @@ -99,6 +121,42 @@ HRESULT WinHttpProvider::PerformAsync( HCCallHandle callHandle, XAsyncBlock* async ) noexcept +{ + // Admission control: only GetGlobalRequestLimit() requests are allowed to reach WinHTTP at + // once, to bound the memory held by in-flight requests (each one costs WinHTTP request and + // connection handles, TLS state, and receive buffers). Requests beyond the cap are queued and + // started as earlier ones complete, so the caller never sees a failure for exceeding the cap. + uint64_t epoch{ 0 }; + { + std::lock_guard lock{ m_lock }; + + if (m_activeRequestCount >= GetGlobalRequestLimit()) + { + HC_TRACE_INFORMATION(HTTPCLIENT, "WinHttpProvider::PerformAsync queueing request, %u already active (limit %u)", + m_activeRequestCount, GetGlobalRequestLimit()); + m_pendingRequests.push_back(PendingRequest{ callHandle, async }); + return S_OK; + } + + ++m_activeRequestCount; + epoch = m_requestEpoch; + } + + HRESULT hr = StartRequest(callHandle, async, epoch); + if (FAILED(hr)) + { + // The request never reached WinHTTP, so it will never complete and would otherwise leak + // its slot. Release it and let any queued request take it. + OnRequestCompleted(epoch); + } + return hr; +} + +HRESULT WinHttpProvider::StartRequest( + HCCallHandle callHandle, + XAsyncBlock* async, + uint64_t epoch +) noexcept { // Get Security information for the call auto getSecurityInfoResult = GetSecurityInformation(callHandle->url.data()); @@ -110,7 +168,16 @@ HRESULT WinHttpProvider::PerformAsync( std::unique_lock lock{ m_lock }; #if HC_PLATFORM == HC_PLATFORM_GDK - if (!m_networkInitialized) + // Refuse new requests while suspended: PLM requires every network resource to be destroyed + // before the process is snapshotted, so nothing may be started until Resume. + // + // Note there is deliberately no "is the network initialized" check here. That gate used to + // consult a cached XNetworking connectivity hint, which could latch false permanently when the + // change notification was never delivered (observed on Steam Deck under Proton after an + // offline->online transition), leaving every request failing with E_HC_NETWORK_NOT_INITIALIZED + // for the life of the process. WinHTTP is the authority on whether a request can go out, so we + // let it try and return a real, retryable error instead. + if (m_isSuspended) { return E_HC_NETWORK_NOT_INITIALIZED; } @@ -123,8 +190,74 @@ HRESULT WinHttpProvider::PerformAsync( // Store weak reference to connection so we can close it if it is still active on shutdown m_connections.push_back(initConnectionResult.Payload()); + // Release the concurrency slot when the request finishes, however it finishes. Safe to capture + // `this`: the provider outlives its connections, which it force-closes in CloseAllConnections + // during both suspend and destruction. + initConnectionResult.Payload()->SetRequestCompletedCallback([this, epoch]() { OnRequestCompleted(epoch); }); + + // Unlock before starting: HttpCallPerformAsync can complete synchronously, which re-enters + // OnRequestCompleted and would deadlock on the non-recursive m_lock. + auto connection = initConnectionResult.Payload(); + lock.unlock(); + // WinHttpConnection manages its own lifetime from here - return initConnectionResult.Payload()->HttpCallPerformAsync(async); + return connection->HttpCallPerformAsync(async); +} + +void WinHttpProvider::OnRequestCompleted(uint64_t epoch) noexcept +{ + // Promote at most one queued request per completion, matching the slot that was just freed. + // Started outside the lock: HttpCallPerformAsync can complete synchronously and re-enter + // OnRequestCompleted, which would deadlock on the non-recursive m_lock. + for (;;) + { + PendingRequest next{}; + uint64_t nextEpoch{ 0 }; + { + std::lock_guard lock{ m_lock }; + + // CloseAllConnections already reclaimed every slot reserved in this epoch, so this + // release refers to a slot that no longer exists. Dropping it is what keeps the + // unsigned count from underflowing and wedging admission control permanently. + if (epoch != m_requestEpoch) + { + return; + } + + // Defensive: the epoch check above should make a zero count unreachable here, but the + // consequence of being wrong is a permanent stall, so saturate rather than wrap. + if (m_activeRequestCount > 0) + { + --m_activeRequestCount; + } + + if (m_pendingRequests.empty() || m_activeRequestCount >= GetGlobalRequestLimit()) + { + return; + } + + next = m_pendingRequests.front(); + m_pendingRequests.pop_front(); + ++m_activeRequestCount; + nextEpoch = m_requestEpoch; + } + + HRESULT hr = StartRequest(next.callHandle, next.async, nextEpoch); + if (SUCCEEDED(hr)) + { + return; + } + + // Complete the failed request for the caller, then loop to release its slot and give the + // next queued request a chance. Without this the caller would wait forever on a request + // that never reached WinHTTP. + HC_TRACE_ERROR_HR(HTTPCLIENT, hr, "WinHttpProvider: queued request failed to start"); + HCHttpCallResponseSetNetworkErrorCode(next.callHandle, hr, static_cast(hr)); + XAsyncComplete(next.async, S_OK, 0); + + // The next iteration releases the slot just reserved above, which belongs to nextEpoch. + epoch = nextEpoch; + } } HRESULT WinHttpProvider::SetGlobalProxy(_In_ String const& proxyUri) noexcept @@ -165,7 +298,9 @@ HRESULT WinHttpProvider::ConnectAsync( std::unique_lock lock{ m_lock }; #if HC_PLATFORM == HC_PLATFORM_GDK - if (!m_networkInitialized) + // See the equivalent comment in StartRequest: suspend blocks new work, but there is + // deliberately no cached network-initialized gate here. + if (m_isSuspended) { return E_HC_NETWORK_NOT_INITIALIZED; } @@ -230,38 +365,28 @@ HRESULT WinHttpProvider::Disconnect( } #endif //!HC_NOWEBSOCKETS -HRESULT WinHttpProvider::CloseAllConnections() +HRESULT WinHttpProvider::CloseAllConnections(DWORD timeoutMs) { // Should set result to HRESULT_FROM_WIN32(PROCESS_SUSPEND_RESUME) - struct CloseContext - { - ~CloseContext() - { - if (connectionsClosedEvent) - { - CloseHandle(connectionsClosedEvent); - } - } - - HANDLE connectionsClosedEvent; - std::atomic openConnections; + // Heap allocated and shared with the completion callback rather than living on this stack + // frame. The wait below is bounded, so a connection can report closed after this function has + // returned; a stack-allocated context would be freed memory by then. + auto closeContext = std::allocate_shared(http_stl_allocator{}); - } closeContext; - - closeContext.connectionsClosedEvent = CreateEvent(nullptr, TRUE, FALSE, nullptr); - if (closeContext.connectionsClosedEvent == nullptr) + closeContext->connectionsClosedEvent = CreateEvent(nullptr, TRUE, FALSE, nullptr); + if (closeContext->connectionsClosedEvent == nullptr) { return HRESULT_FROM_WIN32(GetLastError()); } - auto connectionClosedCallback = [&closeContext]() + auto connectionClosedCallback = [closeContext]() { - HC_TRACE_VERBOSE(HTTPCLIENT, "WinHttpProvider::Connection Closed, %llu remaining", closeContext.openConnections - 1); + HC_TRACE_VERBOSE(HTTPCLIENT, "WinHttpProvider::Connection Closed, %llu remaining", closeContext->openConnections - 1); - if (--closeContext.openConnections == 0) + if (--closeContext->openConnections == 0) { - SetEvent(closeContext.connectionsClosedEvent); + SetEvent(closeContext->connectionsClosedEvent); } }; @@ -280,14 +405,20 @@ HRESULT WinHttpProvider::CloseAllConnections() m_connections.clear(); } - closeContext.openConnections = connections.size(); - if (closeContext.openConnections > 0) + closeContext->openConnections = connections.size(); + if (closeContext->openConnections > 0) { for (auto& connection : connections) { assert(connection); if (connection) { + // Drop the budget callback before closing. These connections are about to be torn + // down as a group and the budget is reset in bulk below, so per-connection release + // is both unnecessary and unsafe here: a connection can outlive this call and would + // then invoke a callback capturing a provider that may already be destroyed. + connection->SetRequestCompletedCallback(nullptr); + HRESULT hr = connection->Close(connectionClosedCallback); if (FAILED(hr)) { @@ -295,7 +426,83 @@ HRESULT WinHttpProvider::CloseAllConnections() } } } - WaitForSingleObject(closeContext.connectionsClosedEvent, INFINITE); + // Bounded on the PLM suspend path, where the title has a short budget to comply before the + // OS terminates it: a connection that fails to report closed in time must not turn a slow + // teardown into a watchdog kill. The close context is shared with the callback, so a late + // report after this point is still safe. Shutdown passes INFINITE instead, because there + // the wait is what guarantees no WinHTTP callback can still run against connections that + // allocate through the libHttpClient allocator after the provider and singleton are gone. + if (WaitForSingleObject(closeContext->connectionsClosedEvent, timeoutMs) == WAIT_TIMEOUT) + { + HC_TRACE_ERROR(HTTPCLIENT, "WinHttpProvider::CloseAllConnections timed out after %ums with %llu connection(s) still open", + timeoutMs, static_cast(closeContext->openConnections.load())); + + // Those connections have already been dropped from m_connections and cannot be closed + // again, so retain the context they were given. The shutdown path drains it below, so + // their eventual close is still observed rather than being lost entirely. + std::lock_guard lock{ m_lock }; + m_pendingCloseContexts.push_back(closeContext); + } + } + + // Drain contexts left behind by earlier timed-out closes. Shutdown only. + // + // These exist to protect provider destruction: the destructor closes the WinHTTP session + // handles a straggler may still be using. Suspend destroys nothing, so draining there buys + // nothing and costs real time - each retained context would burn its full timeout in sequence, + // and these are by definition the connections least likely to ever report, so worst case adds + // N x the timeout to the suspend budget. That is precisely the watchdog kill the bounded wait + // above exists to avoid, and it would delay the E_ABORT completions below by the same amount. + // + // Bounded even here rather than INFINITE: a connection that already missed one deadline is the + // one most likely never to report, and blocking HCCleanup forever would be worse than the race + // this closes. The unbounded wait above still covers connections closed by this call, which is + // the case that actually protects the session handles. Best-effort second chance, not a + // guarantee. + if (timeoutMs == INFINITE) + { + constexpr DWORD c_retainedCloseTimeoutMs = 5000; + + http_internal_vector> pendingCloseContexts; + { + std::lock_guard lock{ m_lock }; + pendingCloseContexts.swap(m_pendingCloseContexts); + } + + for (auto& pendingContext : pendingCloseContexts) + { + if (WaitForSingleObject(pendingContext->connectionsClosedEvent, c_retainedCloseTimeoutMs) == WAIT_TIMEOUT) + { + HC_TRACE_ERROR(HTTPCLIENT, "WinHttpProvider::CloseAllConnections: %llu connection(s) from an earlier close still open", + static_cast(pendingContext->openConnections.load())); + + std::lock_guard lock{ m_lock }; + m_pendingCloseContexts.push_back(pendingContext); + } + } + } + + // Every connection is gone, so nothing is in flight. Reset the budget rather than relying on + // per-request release, and fail anything still queued: those requests never reached WinHTTP and + // there is no longer a completion that would start them, so callers must not be left waiting. + http_internal_list abandoned; + { + std::lock_guard lock{ m_lock }; + m_activeRequestCount = 0; + + // Invalidate every slot reserved so far. A request that reserved a slot before this reset + // can still run its completion afterwards (it may have been mid-flight, or its connection + // may already have been destroyed and so skipped by the callback-clearing loop above); + // without this its release would decrement a count that was just zeroed and underflow it. + ++m_requestEpoch; + + abandoned.swap(m_pendingRequests); + } + + for (auto& pending : abandoned) + { + HCHttpCallResponseSetNetworkErrorCode(pending.callHandle, E_ABORT, static_cast(E_ABORT)); + XAsyncComplete(pending.async, S_OK, 0); } return S_OK; @@ -364,7 +571,8 @@ Result WinHttpProvider::GetHSession(uint32_t securityProtocolFlags, c #endif std::lock_guard lock(m_lock); - auto iter = m_hSessions.find(securityProtocolFlags); + SessionKey const sessionKey{ securityProtocolFlags, isHttps }; + auto iter = m_hSessions.find(sessionKey); if (iter != m_hSessions.end()) { HINTERNET hSession = iter->second; @@ -495,7 +703,7 @@ Result WinHttpProvider::GetHSession(uint32_t securityProtocolFlags, c (void)SetGlobalProxyForHSession(hSession, m_globalProxy.c_str()); } - m_hSessions[securityProtocolFlags] = hSession; + m_hSessions[sessionKey] = hSession; return hSession; } @@ -595,15 +803,25 @@ void WinHttpProvider::Suspend() { HC_TRACE_INFORMATION(HTTPCLIENT, "WinHttpProvider::Suspend"); + // Held across the entire sequence, including the blocking drain in CloseAllConnections, so a + // concurrent Resume or provider teardown cannot interleave with a suspend in progress. + std::lock_guard suspendLock{ m_suspendLock }; + { std::lock_guard lock{ m_lock }; - assert(!m_isSuspended); + if (m_isSuspended) + { + HC_TRACE_VERBOSE(HTTPCLIENT, "WinHttpProvider::Suspend called while already suspended, ignoring"); + return; + } m_isSuspended = true; - m_networkInitialized = false; } - HRESULT hr = CloseAllConnections(); + // Bounded on suspend: exceeding the platform's suspend budget gets the title killed by the + // watchdog, which is worse than proceeding with a connection that has not reported closed. + constexpr DWORD c_suspendCloseTimeoutMs = 2000; + HRESULT hr = CloseAllConnections(c_suspendCloseTimeoutMs); if (FAILED(hr)) { HC_TRACE_ERROR_HR(HTTPCLIENT, hr, "WinHttpProvider::CloseAllConnections failed during suspend, continuing with suspend sequence"); @@ -624,49 +842,18 @@ void WinHttpProvider::Resume() { HC_TRACE_INFORMATION(HTTPCLIENT, "WinHttpProvider::Resume"); - std::unique_lock lock{ m_lock }; - - assert(m_isSuspended); - m_isSuspended = false; - - lock.unlock(); - - // Force a query of network state since we've ignored notifications during suspend - NetworkConnectivityChangedCallback(this, nullptr); -} - -void WinHttpProvider::NetworkConnectivityChangedCallback(void* context, const XNetworkingConnectivityHint* /*hint*/) -{ - assert(context); - auto provider = static_cast(context); + // Blocks until any in-progress Suspend has fully completed, so resume can never race ahead of + // the teardown and let new requests start against half-destroyed state. + std::lock_guard suspendLock{ m_suspendLock }; - std::lock_guard lock{ provider->m_lock }; + std::lock_guard lock{ m_lock }; - // Ignore network connectivity changes if we are suspended - if (!provider->m_isSuspended) + if (!m_isSuspended) { - if (XGameRuntimeIsFeatureAvailable(XGameRuntimeFeature::XNetworking)) - { - // Always requery the latest network connectivity hint rather than relying on the passed parameter in case this is a stale notification - XNetworkingConnectivityHint hint{}; - HRESULT hr = XNetworkingGetConnectivityHint(&hint); - if (SUCCEEDED(hr)) - { - HC_TRACE_INFORMATION(HTTPCLIENT, "NetworkConnectivityChangedCallback, hint.networkInitialized=%d", hint.networkInitialized); - provider->m_networkInitialized = hint.networkInitialized; - } - else - { - HC_TRACE_ERROR(HTTPCLIENT, "Unable to get NetworkConnectivityHint, setting m_networkInitialized=false"); - provider->m_networkInitialized = false; - } - } - else - { - // Fallback to default network state if XNetworking is not available - provider->m_networkInitialized = true; - } + HC_TRACE_VERBOSE(HTTPCLIENT, "WinHttpProvider::Resume called while not suspended, ignoring"); + return; } + m_isSuspended = false; } void WinHttpProvider::AppStateChangedCallback(BOOLEAN isSuspended, void* context) diff --git a/Source/HTTP/WinHttp/winhttp_provider.h b/Source/HTTP/WinHttp/winhttp_provider.h index 5e5410ef4..b9fa41252 100644 --- a/Source/HTTP/WinHttp/winhttp_provider.h +++ b/Source/HTTP/WinHttp/winhttp_provider.h @@ -70,6 +70,11 @@ class WinHttpProvider XAsyncBlock* async ) noexcept; + // Global cap on the number of HTTP requests allowed to be in flight against WinHTTP at once. + // Requests beyond the cap are queued and started as earlier requests complete, so callers may + // enqueue as many as they like. The limit itself is process-wide state owned by global.h + // (xbox::httpclient::GetGlobalRequestLimit), since it may be set before the provider exists. + HRESULT SetGlobalProxy( _In_ String const& proxyUri ) noexcept; @@ -108,7 +113,19 @@ class WinHttpProvider private: WinHttpProvider() = default; - HRESULT CloseAllConnections(); + // timeoutMs bounds the wait for connections to report closed. Suspend passes a short budget so + // a stuck connection cannot trigger the PLM watchdog; shutdown passes INFINITE because the wait + // is what prevents callbacks from outliving the provider. + HRESULT CloseAllConnections(DWORD timeoutMs); + + // Starts a request against WinHTTP immediately, bypassing the admission check. Callers must + // already hold a reserved slot in m_activeRequestCount, reserved during epoch `epoch`. + HRESULT StartRequest(HCCallHandle callHandle, XAsyncBlock* async, uint64_t epoch) noexcept; + + // Called when an admitted request finishes. Releases its slot and promotes queued requests. + // `epoch` is the value of m_requestEpoch when the slot was reserved; a release from an earlier + // epoch is ignored because CloseAllConnections already reclaimed those slots in bulk. + void OnRequestCompleted(uint64_t epoch) noexcept; Result GetSecurityInformation(const char* url); Result GetHSession(uint32_t securityProtocolFlags, const char* url); @@ -121,25 +138,83 @@ class WinHttpProvider http_internal_string m_globalProxy; std::mutex m_lock; - // Maintain a WinHttpSession for each unique security protocol flags - http_internal_map m_hSessions; + // Maintain a WinHttpSession for each unique (security protocol flags, secure scheme) pair. + // + // `isSecure` is part of the key because WinHTTP sessions cannot be reused across TLS and + // non-TLS connections. TLS-enabled sessions are restricted with WINHTTP_FLAG_SECURE_DEFAULTS + // which then prevents those sessions from being used with insecure WS and HTTP schemes. + // Keying on the protocol flags alone let whichever ran first win the cache slot, so an http:// + // or ws:// request that followed an https:// request reused the secure-defaults session and + // failed in WinHttpOpenRequest with ERROR_ACCESS_DENIED. + struct SessionKey + { + uint32_t securityProtocolFlags; + bool isSecure; + + bool operator<(SessionKey const& other) const + { + if (securityProtocolFlags != other.securityProtocolFlags) + { + return securityProtocolFlags < other.securityProtocolFlags; + } + return isSecure < other.isSecure; + } + }; + http_internal_map m_hSessions; + + // Shared with the connection-closed callbacks so a connection that reports closed after a + // bounded wait has already returned still has a valid context to signal. Defined in the .cpp. + struct CloseContext; + + // Contexts from earlier CloseAllConnections calls whose bounded wait expired with connections + // still open. Those connections were dropped from m_connections and cannot be closed a second + // time (WinHttpConnection::Close is once-only and returns E_UNEXPECTED without invoking the + // callback), so waiting on the context they were originally given is the only way to observe + // them finishing. Only the shutdown path drains these, and only for a bounded time - not + // INFINITE - so a connection that already missed one deadline cannot hang HCCleanup forever. + // Best-effort: it narrows the window where a straggler outlives the provider and the WinHTTP + // session handles it is still using, rather than closing it entirely. + http_internal_vector> m_pendingCloseContexts; // Track WinHttpConnections so that we can close them on shutdown/suspend http_internal_list> m_connections; + // Requests admitted to WinHTTP but not yet completed. Bounded by GetGlobalRequestLimit(). + uint32_t m_activeRequestCount{ 0 }; + + // Incremented whenever CloseAllConnections reclaims every slot at once. A slot reserved before + // that reset must not be released again afterwards: the count is unsigned, so the stray + // decrement would underflow to UINT32_MAX and make m_activeRequestCount >= the limit + // permanently true, queueing every subsequent request forever. Releases carry the epoch they + // were reserved in and are dropped if it no longer matches. + uint64_t m_requestEpoch{ 0 }; + + // Requests the caller has submitted that are waiting for a free slot. Unbounded by design: + // titles may queue as many requests as they like, only concurrency is capped. FIFO, so a + // queued request cannot be starved by later arrivals. + struct PendingRequest + { + HCCallHandle callHandle; + XAsyncBlock* async; + }; + http_internal_list m_pendingRequests; + #if HC_PLATFORM == HC_PLATFORM_GDK public: // For testing purposes only void Suspend(); void Resume(); private: - static void CALLBACK NetworkConnectivityChangedCallback(void* context, const XNetworkingConnectivityHint* hint); static void CALLBACK AppStateChangedCallback(BOOLEAN isSuspended, void* context); - bool m_networkInitialized{ false }; bool m_isSuspended{ false }; - XTaskQueueRegistrationToken m_networkConnectivityChangedToken{ 0 }; PAPPSTATE_REGISTRATION m_appStateChangedToken{ nullptr }; + + // Serializes the whole suspend sequence against resume and against provider destruction. + // Suspend drops m_lock while it blocks waiting for connections to close, so m_lock alone cannot + // keep a concurrent Resume (or a destructor running CloseAllConnections) from interleaving with + // a suspend that is still in progress. + std::mutex m_suspendLock; #endif }; diff --git a/Source/Platform/GDK/PlatformComponents_GDK.cpp b/Source/Platform/GDK/PlatformComponents_GDK.cpp index 2bc512b61..6a25ac13a 100644 --- a/Source/Platform/GDK/PlatformComponents_GDK.cpp +++ b/Source/Platform/GDK/PlatformComponents_GDK.cpp @@ -1,28 +1,12 @@ #include "pch.h" #include "Platform/PlatformComponents.h" -#include "HTTP/Curl/CurlProvider.h" #include "HTTP/WinHttp/winhttp_provider.h" #if !defined(HC_NOWEBSOCKETS) && defined(HC_ENABLE_WEBSOCKET_COMPRESSION) #include "HTTP/WinHttp/winhttp_websocket_hybrid.h" #endif -#if HC_PLATFORM == HC_PLATFORM_GDK -#include "XSystem.h" -#endif - NAMESPACE_XBOX_HTTP_CLIENT_BEGIN -// On GDK, desktop PC is the special case. Treat every non-PC device type as console -// so new console device types continue to take the safer console path by default. -static bool IsRunningOnXboxConsole() -{ -#if HC_PLATFORM == HC_PLATFORM_GDK - return XSystemGetDeviceType() != XSystemDeviceType::Pc; -#else - return false; -#endif -} - #ifndef HC_NOWEBSOCKETS static bool IsGdkXboxCompressionWebSocketProviderEnabled() noexcept { @@ -33,14 +17,13 @@ static bool IsGdkXboxCompressionWebSocketProviderEnabled() noexcept #endif } -static HRESULT InitializeGdkWebSocketProviders(PlatformComponents& components, bool enableCompressionWebSocketProvider) +// Selects the default WebSocket provider. Shares the caller's WinHttpProvider instance so the +// process has exactly one provider, and therefore one PLM registration and one suspend drain. +static HRESULT InitializeGdkWebSocketProviders( + PlatformComponents& components, + bool enableCompressionWebSocketProvider, + SharedPtr const& sharedWinHttpProvider) { - auto initWinHttpResult = WinHttpProvider::Initialize(); - RETURN_IF_FAILED(initWinHttpResult.hr); - - auto winHttpProvider = initWinHttpResult.ExtractPayload(); - auto sharedWinHttpProvider = SharedPtr{ winHttpProvider.release(), std::move(winHttpProvider.get_deleter()), http_stl_allocator{} }; - #if defined(HC_ENABLE_WEBSOCKET_COMPRESSION) if (enableCompressionWebSocketProvider) { @@ -61,50 +44,21 @@ HRESULT PlatformInitialize(PlatformComponents& components, HCInitArgs* initArgs) // We don't expect initArgs on GDK RETURN_HR_IF(E_INVALIDARG, initArgs); - // Detect runtime platform to choose appropriate HTTP provider - if (IsRunningOnXboxConsole()) - { - HC_TRACE_INFORMATION(HTTPCLIENT, "PlatformInitialize: Detected Xbox console, using XCurl for HTTP"); - - // Use XCurl for Xbox console with full PLM support - auto initXCurlResult = CurlProvider::Initialize(); - RETURN_IF_FAILED(initXCurlResult.hr); + HC_TRACE_INFORMATION(HTTPCLIENT, "PlatformInitialize: Using WinHTTP for HTTP"); - components.HttpProvider = initXCurlResult.ExtractPayload(); + auto initWinHttpResult = WinHttpProvider::Initialize(); + RETURN_IF_FAILED(initWinHttpResult.hr); -#ifndef HC_NOWEBSOCKETS - // For Xbox consoles with XCurl HTTP, still use WinHttp for the default WebSocket path. - bool const enableCompressionWebSocketProvider = IsGdkXboxCompressionWebSocketProviderEnabled(); - if (!enableCompressionWebSocketProvider) - { - HC_TRACE_INFORMATION(HTTPCLIENT, "PlatformInitialize: Xbox console compression WebSocket provider is disabled by build policy"); - } - RETURN_IF_FAILED(InitializeGdkWebSocketProviders(components, enableCompressionWebSocketProvider)); -#endif - } - else - { - HC_TRACE_INFORMATION(HTTPCLIENT, "PlatformInitialize: Detected non-console platform. Using WinHTTP for HTTP"); - - // Use WinHTTP for non-console platforms - auto initWinHttpResult = WinHttpProvider::Initialize(); - RETURN_IF_FAILED(initWinHttpResult.hr); + auto winHttpProvider = initWinHttpResult.ExtractPayload(); - auto winHttpProvider = initWinHttpResult.ExtractPayload(); + // Use the same WinHttpProvider instance for both HTTP and the default WebSocket path. + auto sharedWinHttpProvider = SharedPtr{ winHttpProvider.release(), std::move(winHttpProvider.get_deleter()), http_stl_allocator{} }; - // Use the same WinHttpProvider instance for both HTTP and the default WebSocket path. - auto sharedWinHttpProvider = SharedPtr{ winHttpProvider.release(), std::move(winHttpProvider.get_deleter()), http_stl_allocator{} }; - - components.HttpProvider = http_allocate_unique(sharedWinHttpProvider); + components.HttpProvider = http_allocate_unique(sharedWinHttpProvider); #ifndef HC_NOWEBSOCKETS -#if defined(HC_ENABLE_WEBSOCKET_COMPRESSION) - components.WebSocketProvider = http_allocate_unique(sharedWinHttpProvider); -#else - components.WebSocketProvider = http_allocate_unique(sharedWinHttpProvider); -#endif + RETURN_IF_FAILED(InitializeGdkWebSocketProviders(components, IsGdkXboxCompressionWebSocketProviderEnabled(), sharedWinHttpProvider)); #endif - } return S_OK; } diff --git a/Source/Platform/Linux/PlatformComponents_Linux.cpp b/Source/Platform/Linux/PlatformComponents_Linux.cpp index 34a35406e..17220d54d 100644 --- a/Source/Platform/Linux/PlatformComponents_Linux.cpp +++ b/Source/Platform/Linux/PlatformComponents_Linux.cpp @@ -10,11 +10,11 @@ HRESULT PlatformInitialize(PlatformComponents& components, HCInitArgs* initArgs) // We don't expect initArgs on linux RETURN_HR_IF(E_INVALIDARG, initArgs); - // XCurl will be used for HTTP - auto initXCurlResult = CurlProvider::Initialize(); - RETURN_IF_FAILED(initXCurlResult.hr); + // libcurl will be used for HTTP + auto initCurlResult = CurlProvider::Initialize(); + RETURN_IF_FAILED(initCurlResult.hr); - components.HttpProvider = initXCurlResult.ExtractPayload(); + components.HttpProvider = initCurlResult.ExtractPayload(); #ifndef HC_NOWEBSOCKETS // Websocketpp will be used for WebSockets diff --git a/Tests/UnitTests/Tests/AsyncBlockTests.cpp b/Tests/UnitTests/Tests/AsyncBlockTests.cpp index 6ba9af422..aa0c59622 100644 --- a/Tests/UnitTests/Tests/AsyncBlockTests.cpp +++ b/Tests/UnitTests/Tests/AsyncBlockTests.cpp @@ -399,9 +399,24 @@ DEFINE_TEST_CLASS(AsyncBlockTests) AsyncBlockTests::~AsyncBlockTests() { - VERIFY_ARE_EQUAL(s_AsyncLibGlobalStateCount, (DWORD)0); + // Completion of an async op (XAsyncGetStatus no longer returning E_PENDING) does not mean + // its AsyncState has been reclaimed yet: the final release happens on the queue thread + // slightly later. Poll rather than sampling the count immediately, otherwise this races + // with any test whose last operation completed just before it returned. + for (int i = 0; i < 500 && s_AsyncLibGlobalStateCount != 0; i++) + { + Sleep(10); + } + + DWORD leakedState = static_cast(s_AsyncLibGlobalStateCount.load()); + + // Tear the queue down before verifying. VERIFY_ARE_EQUAL throws on failure, so asserting + // first would skip this cleanup and leak the task queue, which then fails the global + // XTaskQueueUninitialize leak check in every subsequent TaskQueueTests case. XTaskQueueTerminate(queue, true, nullptr, nullptr); XTaskQueueCloseHandle(queue); + + VERIFY_ARE_EQUAL(leakedState, (DWORD)0); } DEFINE_TEST_CASE(VerifySimpleAsyncCall) diff --git a/Tests/UnitTests/Tests/GlobalTests.cpp b/Tests/UnitTests/Tests/GlobalTests.cpp index 0f5272442..3584f0cf1 100644 --- a/Tests/UnitTests/Tests/GlobalTests.cpp +++ b/Tests/UnitTests/Tests/GlobalTests.cpp @@ -36,6 +36,12 @@ static bool g_gotCall = false; NAMESPACE_XBOX_HTTP_CLIENT_TEST_BEGIN +// The default request limit is device-dependent: 12 on Xbox consoles, unlimited everywhere else. +// This test project builds for Win32/UWP, so the expected default here is unlimited. Kept at +// namespace scope rather than inside the test class because a non-static member cannot be +// constexpr, and a static one would be odr-used by VERIFY_ARE_EQUAL under C++14. +constexpr uint32_t c_expectedDefaultRequestLimit = UINT32_MAX; + DEFINE_TEST_CLASS(GlobalTests) { public: @@ -296,6 +302,113 @@ DEFINE_TEST_CLASS(GlobalTests) HCCleanup(); XTaskQueueCloseHandle(queue); } + + // ------------------------------------------------------------------------ + // Global request limit + // + // These cover the API contract only. Enforcement (admission control and the pending + // queue) lives in WinHttpProvider, which this test project does not compile -- it builds + // PlatformComponents_Generic instead -- so concurrency behavior is covered by the + // device-side scenarios in PlayFab.C\Test\GameTestScenarios\lhc-xbox rather than here. + // ------------------------------------------------------------------------ + + // Restores the shipping default so a failure part-way through a test cannot leak a custom + // limit into later tests. The limit is process-wide and deliberately survives HCCleanup, + // so nothing else resets it. + struct RequestLimitRestorer + { + ~RequestLimitRestorer() { HCSettingsSetGlobalRequestLimit(0); } + }; + + DEFINE_TEST_CASE(TestGlobalRequestLimitDefault) + { + DEFINE_TEST_CASE_PROPERTIES(TestGlobalRequestLimitDefault); + RequestLimitRestorer restore; + + // Readable before HCInitialize: a title needs to size the cap before issuing requests, + // so these APIs must not require initialization. + VERIFY_ARE_EQUAL(HCIsInitialized(), false); + + uint32_t limit{ 0 }; + VERIFY_SUCCEEDED(HCSettingsGetGlobalRequestLimit(&limit)); + VERIFY_ARE_EQUAL(c_expectedDefaultRequestLimit, limit); + } + + DEFINE_TEST_CASE(TestGlobalRequestLimitRoundTrip) + { + DEFINE_TEST_CASE_PROPERTIES(TestGlobalRequestLimitRoundTrip); + RequestLimitRestorer restore; + + VERIFY_SUCCEEDED(HCSettingsSetGlobalRequestLimit(4)); + + uint32_t limit{ 0 }; + VERIFY_SUCCEEDED(HCSettingsGetGlobalRequestLimit(&limit)); + VERIFY_ARE_EQUAL(4u, limit); + + VERIFY_SUCCEEDED(HCSettingsSetGlobalRequestLimit(64)); + VERIFY_SUCCEEDED(HCSettingsGetGlobalRequestLimit(&limit)); + VERIFY_ARE_EQUAL(64u, limit); + + // A single request in flight is a legitimate configuration, so 1 must be preserved + // rather than treated as a degenerate value. + VERIFY_SUCCEEDED(HCSettingsSetGlobalRequestLimit(1)); + VERIFY_SUCCEEDED(HCSettingsGetGlobalRequestLimit(&limit)); + VERIFY_ARE_EQUAL(1u, limit); + } + + DEFINE_TEST_CASE(TestGlobalRequestLimitZeroRestoresDefault) + { + DEFINE_TEST_CASE_PROPERTIES(TestGlobalRequestLimitZeroRestoresDefault); + RequestLimitRestorer restore; + + VERIFY_SUCCEEDED(HCSettingsSetGlobalRequestLimit(3)); + + // A literal cap of zero would park every request forever with no completion able to + // release a slot, wedging the title. 0 therefore means "restore the default". + VERIFY_SUCCEEDED(HCSettingsSetGlobalRequestLimit(0)); + + uint32_t limit{ 0 }; + VERIFY_SUCCEEDED(HCSettingsGetGlobalRequestLimit(&limit)); + VERIFY_ARE_EQUAL(c_expectedDefaultRequestLimit, limit); + } + + DEFINE_TEST_CASE(TestGlobalRequestLimitInvalidArg) + { + DEFINE_TEST_CASE_PROPERTIES(TestGlobalRequestLimitInvalidArg); + + VERIFY_ARE_EQUAL(E_INVALIDARG, HCSettingsGetGlobalRequestLimit(nullptr)); + } + + DEFINE_TEST_CASE(TestGlobalRequestLimitSurvivesInitAndCleanup) + { + DEFINE_TEST_CASE_PROPERTIES(TestGlobalRequestLimitSurvivesInitAndCleanup); + RequestLimitRestorer restore; + + // Set before init: the provider must read the live global rather than snapshotting the + // value when it is constructed. + VERIFY_ARE_EQUAL(HCIsInitialized(), false); + VERIFY_SUCCEEDED(HCSettingsSetGlobalRequestLimit(7)); + + VERIFY_SUCCEEDED(HCInitialize(nullptr)); + + uint32_t limit{ 0 }; + VERIFY_SUCCEEDED(HCSettingsGetGlobalRequestLimit(&limit)); + VERIFY_ARE_EQUAL(7u, limit); + + // Settable while initialized too. + VERIFY_SUCCEEDED(HCSettingsSetGlobalRequestLimit(9)); + VERIFY_SUCCEEDED(HCSettingsGetGlobalRequestLimit(&limit)); + VERIFY_ARE_EQUAL(9u, limit); + + HCCleanup(); + + // Deliberately process-wide rather than part of http_singleton, so it outlives cleanup + // and a title that configures it once at startup does not have to re-apply it after + // every init/cleanup cycle. + VERIFY_ARE_EQUAL(HCIsInitialized(), false); + VERIFY_SUCCEEDED(HCSettingsGetGlobalRequestLimit(&limit)); + VERIFY_ARE_EQUAL(9u, limit); + } }; NAMESPACE_XBOX_HTTP_CLIENT_TEST_END diff --git a/libHttpClient.props b/libHttpClient.props index c8219d0cc..7ea5e88cd 100644 --- a/libHttpClient.props +++ b/libHttpClient.props @@ -90,13 +90,6 @@ - - - - - - - %(AdditionalLibraryDirectories);$(Console_SdkLibPath)