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..c9f8665486 100644 --- a/src/AppInstallerSharedLib/Runtime.cpp +++ b/src/AppInstallerSharedLib/Runtime.cpp @@ -218,4 +218,21 @@ 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; + } }