From bb617f56ef61ac193d6d77fb5f57cbd80dad4ac9 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Mon, 11 May 2026 16:39:11 -0700 Subject: [PATCH 1/2] Small stack size improvements Connect and GetInformation and dynamic choice to include message based on stack size in rethrow --- .../HttpClientHelper.cpp | 16 ++++++- .../Public/winget/RepositorySource.h | 2 +- .../RepositorySource.cpp | 2 +- .../Rest/RestClient.cpp | 46 +++++++++---------- .../Public/winget/Runtime.h | 4 ++ src/AppInstallerSharedLib/Runtime.cpp | 15 ++++++ 6 files changed, 58 insertions(+), 27 deletions(-) diff --git a/src/AppInstallerCommonCore/HttpClientHelper.cpp b/src/AppInstallerCommonCore/HttpClientHelper.cpp index 439171ce81..5d5c783649 100644 --- a/src/AppInstallerCommonCore/HttpClientHelper.cpp +++ b/src/AppInstallerCommonCore/HttpClientHelper.cpp @@ -267,6 +267,20 @@ namespace AppInstaller::Http toThrow = HRESULT_FROM_WIN32(errorValue); } - THROW_HR_MSG(toThrow, "%hs", exception.what()); + // Ensure that sufficient stack space remains to include the message. + // We have seen rare crashes due to running out of stack space in this path + // so we will only include the message if there is enough space. + // PrintLoggingMessage usage in WIL always creates a 4K stack buffer (2K of wchar_t), + // so we leave some on top of that as well as the buffer is kept alive into further calls. + static constexpr size_t s_msgMinimum = 5 * 1024; + + if (Runtime::IsStackAvailable(s_msgMinimum)) + { + THROW_HR_MSG(toThrow, "%hs", exception.what()); + } + else + { + THROW_HR(toThrow); + } } } diff --git a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h index 9b20f55302..f26708c23b 100644 --- a/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h +++ b/src/AppInstallerRepositoryCore/Public/winget/RepositorySource.h @@ -272,7 +272,7 @@ namespace AppInstaller::Repository std::string GetIdentifier() const; // Get the source's configuration details from settings. - SourceDetails GetDetails() const; + const SourceDetails& GetDetails() const; // Get the source's information. SourceInformation GetInformation() const; diff --git a/src/AppInstallerRepositoryCore/RepositorySource.cpp b/src/AppInstallerRepositoryCore/RepositorySource.cpp index 5fdcccea0b..a6473680bd 100644 --- a/src/AppInstallerRepositoryCore/RepositorySource.cpp +++ b/src/AppInstallerRepositoryCore/RepositorySource.cpp @@ -621,7 +621,7 @@ namespace AppInstaller::Repository } } - SourceDetails Source::GetDetails() const + const SourceDetails& Source::GetDetails() const { if (m_source) { diff --git a/src/AppInstallerRepositoryCore/Rest/RestClient.cpp b/src/AppInstallerRepositoryCore/Rest/RestClient.cpp index ffc9609a2f..2c5f592c40 100644 --- a/src/AppInstallerRepositoryCore/Rest/RestClient.cpp +++ b/src/AppInstallerRepositoryCore/Rest/RestClient.cpp @@ -142,36 +142,34 @@ namespace AppInstaller::Repository::Rest // Check the cache for a valid information entry RestInformationCache informationCache; - std::optional cachedInformation = informationCache.Get(endpoint, customHeader, caller); + std::optional result = informationCache.Get(endpoint, customHeader, caller); - if (cachedInformation) + if (!result) { - return std::move(cachedInformation).value(); - } - - // Not in cache, make REST call to retrieve it - auto headers = GetHeaders(customHeader, caller); - CacheControlPolicy cacheControl; - - std::optional response = helper.HandleGet( - endpoint, - headers, - {}, - [&](const web::http::http_response& httpResponse) - { - cacheControl = CacheControlPolicy{ httpResponse.headers().cache_control() }; - return Http::HttpClientHelper::HttpResponseHandlerResult{ std::nullopt, true }; - }); + // Not in cache, make REST call to retrieve it + auto headers = GetHeaders(customHeader, caller); + CacheControlPolicy cacheControl; + + std::optional response = helper.HandleGet( + endpoint, + headers, + {}, + [&](const web::http::http_response& httpResponse) + { + cacheControl = CacheControlPolicy{ httpResponse.headers().cache_control() }; + return Http::HttpClientHelper::HttpResponseHandlerResult{ std::nullopt, true }; + }); - THROW_HR_IF(APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE, !response); + THROW_HR_IF(APPINSTALLER_CLI_ERROR_UNSUPPORTED_RESTSOURCE, !response); - InformationResponseDeserializer responseDeserializer; - auto result = responseDeserializer.Deserialize(response.value()); + InformationResponseDeserializer responseDeserializer; + result = responseDeserializer.Deserialize(response.value()); - // Cache the information value as requested - informationCache.Cache(endpoint, customHeader, caller, cacheControl, std::move(response).value()); + // Cache the information value as requested + informationCache.Cache(endpoint, customHeader, caller, cacheControl, std::move(response).value()); + } - return result; + return std::move(result).value(); } std::unique_ptr RestClient::GetSupportedInterface( diff --git a/src/AppInstallerSharedLib/Public/winget/Runtime.h b/src/AppInstallerSharedLib/Public/winget/Runtime.h index a8500a56e9..9f29760ce2 100644 --- a/src/AppInstallerSharedLib/Public/winget/Runtime.h +++ b/src/AppInstallerSharedLib/Public/winget/Runtime.h @@ -51,6 +51,10 @@ namespace AppInstaller::Runtime // 3. the token is not already elevated bool IsRunningWithLimitedToken(); + // Determines if the given amount of stack bytes are available. + // If the answer cannot be determined properly, the return value will be `false`. + DECLSPEC_NOINLINE bool IsStackAvailable(size_t bytes); + // Returns true if this is a release build; false if not. inline constexpr bool IsReleaseBuild() { diff --git a/src/AppInstallerSharedLib/Runtime.cpp b/src/AppInstallerSharedLib/Runtime.cpp index b3c6e229a0..967b2cc3bd 100644 --- a/src/AppInstallerSharedLib/Runtime.cpp +++ b/src/AppInstallerSharedLib/Runtime.cpp @@ -218,4 +218,19 @@ namespace AppInstaller::Runtime { return wil::get_token_information() == TokenElevationTypeLimited; } + + DECLSPEC_NOINLINE bool IsStackAvailable(size_t bytes) + { + // https://devblogs.microsoft.com/oldnewthing/20200610-00/?p=103855 + ULONG_PTR low, high; + GetCurrentThreadStackLimits(&low, &high); + auto remaining = reinterpret_cast(&low) - low; + if (remaining > high - low) { + // Choosing to return false instead of failing + return false; + } + ULONG guarantee = 0; + SetThreadStackGuarantee(&guarantee); + return remaining >= bytes + guarantee; + } } From edb9c39bb8136eac1364ba02cb58807780b9c178 Mon Sep 17 00:00:00 2001 From: JohnMcPMS Date: Tue, 12 May 2026 17:34:28 -0700 Subject: [PATCH 2/2] format --- src/AppInstallerSharedLib/Runtime.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/AppInstallerSharedLib/Runtime.cpp b/src/AppInstallerSharedLib/Runtime.cpp index 967b2cc3bd..c9f8665486 100644 --- a/src/AppInstallerSharedLib/Runtime.cpp +++ b/src/AppInstallerSharedLib/Runtime.cpp @@ -225,10 +225,12 @@ namespace AppInstaller::Runtime ULONG_PTR low, high; GetCurrentThreadStackLimits(&low, &high); auto remaining = reinterpret_cast(&low) - low; - if (remaining > high - low) { + if (remaining > high - low) + { // Choosing to return false instead of failing return false; } + ULONG guarantee = 0; SetThreadStackGuarantee(&guarantee); return remaining >= bytes + guarantee;