diff --git a/dev/AppLifecycle/AppInstance.cpp b/dev/AppLifecycle/AppInstance.cpp index 7c4d3cbfe8..be479b461e 100644 --- a/dev/AppLifecycle/AppInstance.cpp +++ b/dev/AppLifecycle/AppInstance.cpp @@ -20,6 +20,11 @@ // 63876312: Prevent AppInstance::GetInstances from registering an invalid process handle. #define WINAPPSDK_CHANGEID_63876312 63876312 +// 61688595: Route AppInstance::GetInstances failures through a noexcept HRESULT worker so a +// failure no longer unwinds through the deep C++/WinRT projection rethrow path that exhausted +// the stack via WIL's FormatMessage-based exception logging chain. +#define WINAPPSDK_CHANGEID_61688595 61688595 + using namespace winrt; using namespace winrt::Windows::Foundation; using namespace winrt::Windows::Foundation::Collections; @@ -304,7 +309,7 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation return s_current.as(); } - IVector AppInstance::GetInstances() + IVector AppInstance::GetInstancesWorker() { // Force the singleton init. GetCurrent(); @@ -365,6 +370,30 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation return instances; } + HRESULT AppInstance::GetInstancesImpl(IVector& instancesOut) noexcept try + { + instancesOut = GetInstancesWorker(); + return S_OK; + } + CATCH_RETURN() + + IVector AppInstance::GetInstances() + { + if (!WinAppSdk::Containment::IsChangeEnabled()) + { + // Pre-61688595 behavior: let the exception propagate straight out of the worker. + return GetInstancesWorker(); + } + + // Delegate to the noexcept GetInstancesImpl worker and rethrow any failure + // HRESULT cleanly via wil::ResultException. This avoids the deep WIL + // FormatMessage-based exception logging chain that caused stack overflow on + // the hot path (Bug 61688595). + IVector instances{ nullptr }; + THROW_IF_FAILED(GetInstancesImpl(instances)); + return instances; + } + AppLifecycle::AppInstance AppInstance::FindOrRegisterForKey(hstring const& key) { // Force the singleton init. diff --git a/dev/AppLifecycle/AppInstance.h b/dev/AppLifecycle/AppInstance.h index beb02af484..9415912573 100644 --- a/dev/AppLifecycle/AppInstance.h +++ b/dev/AppLifecycle/AppInstance.h @@ -55,6 +55,16 @@ namespace winrt::Microsoft::Windows::AppLifecycle::implementation void EnqueueRedirectionRequestId(GUID id); GUID DequeueRedirectionRequestId(); + // Throwing implementation of GetInstances. Called directly when contained change + // 61688595 is disabled, and via GetInstancesImpl when it is enabled. + static winrt::Windows::Foundation::Collections::IVector GetInstancesWorker(); + + // noexcept HRESULT-returning worker for GetInstances. The public GetInstances + // boundary is intentionally thin so a failed HRESULT does not trigger the deep + // C++/WinRT projection rethrow path that exhausted the stack via WIL's + // FormatMessage-based exception logging (Bug 61688595). + static HRESULT GetInstancesImpl(winrt::Windows::Foundation::Collections::IVector& instancesOut) noexcept; + // Named object prefixes used to scope. std::wstring m_moduleName; std::wstring m_processName; diff --git a/dev/AppNotifications/AppNotificationManager.cpp b/dev/AppNotifications/AppNotificationManager.cpp index bcee2ddfc5..cd39aec9f6 100644 --- a/dev/AppNotifications/AppNotificationManager.cpp +++ b/dev/AppNotifications/AppNotificationManager.cpp @@ -27,6 +27,12 @@ #include #include #include +#include + +// 61688595: Route AppNotificationManager::Show failures through a noexcept HRESULT worker so a +// failure no longer unwinds through the deep C++/WinRT projection rethrow path that exhausted +// the stack via WIL's FormatMessage-based exception logging chain. +#define WINAPPSDK_CHANGEID_61688595 61688595 using namespace std::literals; @@ -421,13 +427,8 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation } CATCH_RETURN() - void AppNotificationManager::Show(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification) + void AppNotificationManager::ShowLegacy(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification) { - if (!IsSupported()) - { - return; - } - auto logTelemetry{ AppNotificationTelemetry::Show::Start( g_telemetryHelper, m_appId, @@ -453,6 +454,56 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation logTelemetry.Stop(); } + HRESULT AppNotificationManager::ShowImpl(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification) noexcept try + { + auto logTelemetry{ AppNotificationTelemetry::Show::Start( + g_telemetryHelper, + m_appId, + notification.Payload(), + notification.Tag(), + notification.Group(), + winrt::AppNotificationConferencingConfig::IsCallingPreviewSupported()) }; + + RETURN_HR_IF(WPN_E_NOTIFICATION_POSTED, notification.Id() != 0); + + winrt::com_ptr<::ABI::Microsoft::Internal::ToastNotifications::INotificationProperties> notificationProperties = winrt::make_self(notification); + + winrt::com_ptr<::ABI::Microsoft::Internal::ToastNotifications::INotificationTransientProperties> notificationTransientProperties = winrt::make_self(notification); + + DWORD notificationId = 0; + RETURN_IF_FAILED(ToastNotifications_PostToast(m_appId.c_str(), notificationProperties.get(), notificationTransientProperties.get(), ¬ificationId)); + + RETURN_HR_IF(E_UNEXPECTED, notificationId == 0); + + implementation::AppNotification* notificationImpl = get_self(notification); + notificationImpl->SetNotificationId(notificationId); + + logTelemetry.Stop(); + return S_OK; + } + CATCH_RETURN() + + void AppNotificationManager::Show(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification) + { + if (!IsSupported()) + { + return; + } + + if (!WinAppSdk::Containment::IsChangeEnabled()) + { + // Pre-61688595 behavior: throw directly from the hot path. + ShowLegacy(notification); + return; + } + + // Delegate to the noexcept ShowImpl worker and rethrow any failure HRESULT + // cleanly via wil::ResultException. This avoids the deep WIL + // FormatMessage-based exception logging chain that caused stack overflow on + // the hot path (Bug 61688595). + THROW_IF_FAILED(ShowImpl(notification)); + } + winrt::Windows::Foundation::IAsyncOperation AppNotificationManager::UpdateAsync(winrt::Microsoft::Windows::AppNotifications::AppNotificationProgressData const data, hstring const tag, hstring const group) { if (!IsSupported()) diff --git a/dev/AppNotifications/AppNotificationManager.h b/dev/AppNotifications/AppNotificationManager.h index 7282fc0e59..4e58340ed1 100644 --- a/dev/AppNotifications/AppNotificationManager.h +++ b/dev/AppNotifications/AppNotificationManager.h @@ -58,6 +58,16 @@ namespace winrt::Microsoft::Windows::AppNotifications::implementation void UnregisterHelper(); + // Throwing implementation of Show. Called directly when contained change 61688595 + // is disabled, and mirrored by ShowImpl when it is enabled. + void ShowLegacy(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification); + + // noexcept HRESULT-returning worker for Show. The public Show boundary is + // intentionally thin so a failed HRESULT does not trigger the deep C++/WinRT + // projection rethrow path that exhausted the stack via WIL's FormatMessage-based + // exception logging (Bug 61688595). + HRESULT ShowImpl(winrt::Microsoft::Windows::AppNotifications::AppNotification const& notification) noexcept; + wil::unique_com_class_object_cookie m_notificationComActivatorRegistration; wil::srwlock m_lock; winrt::event m_notificationHandlers; diff --git a/test/AppLifecycle/AppLifecycle.vcxproj b/test/AppLifecycle/AppLifecycle.vcxproj index 1b3c95cc7e..71e78c878b 100644 --- a/test/AppLifecycle/AppLifecycle.vcxproj +++ b/test/AppLifecycle/AppLifecycle.vcxproj @@ -181,6 +181,7 @@ + diff --git a/test/AppLifecycle/FunctionalTests.cpp b/test/AppLifecycle/FunctionalTests.cpp index 3f969ee795..4cf7a94622 100644 --- a/test/AppLifecycle/FunctionalTests.cpp +++ b/test/AppLifecycle/FunctionalTests.cpp @@ -95,6 +95,31 @@ namespace Test::AppLifecycle VERIFY_IS_NOT_NULL(launchArgs); } + TEST_METHOD(GetInstancesIncludesCurrentInstance) + { + // Bug 61688595: GetInstances() is now a thin boundary over the noexcept + // GetInstancesImpl worker (when the contained change is enabled) or a direct + // call into GetInstancesWorker (when it is disabled). Either way the public + // API must still enumerate the current instance. + auto current{ AppInstance::GetCurrent() }; + VERIFY_IS_NOT_NULL(current); + + auto instances{ AppInstance::GetInstances() }; + VERIFY_IS_NOT_NULL(instances); + VERIFY_IS_TRUE(instances.Size() > 0); + + bool foundCurrent{ false }; + for (auto const& instance : instances) + { + if (instance.ProcessId() == ::GetCurrentProcessId()) + { + VERIFY_IS_TRUE(instance.IsCurrent()); + foundCurrent = true; + } + } + VERIFY_IS_TRUE(foundCurrent); + } + TEST_METHOD(GetActivatedEventArgsForFile_Win32) { // Create a named event for communicating with test app. diff --git a/test/AppNotificationTests/BaseTestSuite.cpp b/test/AppNotificationTests/BaseTestSuite.cpp index dddbf38450..ea7f7be193 100644 --- a/test/AppNotificationTests/BaseTestSuite.cpp +++ b/test/AppNotificationTests/BaseTestSuite.cpp @@ -219,6 +219,20 @@ void BaseTestSuite::VerifyShowToast() VERIFY_IS_TRUE(VerifyToastsPosted(toastVector)); } +void BaseTestSuite::VerifyShowToastTwiceFailsWithNotificationPosted() +{ + RegisterWithAppNotificationManager(); + + AppNotification toast{ CreateToastNotification() }; + AppNotificationManager::Default().Show(toast); + VERIFY_ARE_NOT_EQUAL(0u, toast.Id()); + + // Bug 61688595: Show() routes through the noexcept ShowImpl worker (or, when the + // contained change is disabled, the legacy throwing body). Either way, re-showing an + // already-posted notification must surface WPN_E_NOTIFICATION_POSTED to the caller. + VERIFY_THROWS_HR(AppNotificationManager::Default().Show(toast), WPN_E_NOTIFICATION_POSTED); +} + void BaseTestSuite::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup() { RegisterWithAppNotificationManager(); diff --git a/test/AppNotificationTests/BaseTestSuite.h b/test/AppNotificationTests/BaseTestSuite.h index bede46a40f..eb7b799ac3 100644 --- a/test/AppNotificationTests/BaseTestSuite.h +++ b/test/AppNotificationTests/BaseTestSuite.h @@ -27,6 +27,7 @@ class BaseTestSuite void VerifyToastSuppressDisplay(); void VerifyToastExpiresOnReboot(); void VerifyShowToast(); + void VerifyShowToastTwiceFailsWithNotificationPosted(); void VerifyUpdateToastProgressDataUsingValidTagAndValidGroup(); void VerifyUpdateToastProgressDataUsingValidTagAndEmptyGroup(); void VerifyUpdateToastProgressDataUsingEmptyTagAndValidGroup(); diff --git a/test/AppNotificationTests/PackagedTests.cpp b/test/AppNotificationTests/PackagedTests.cpp index f6d299e7bb..56ebee9853 100644 --- a/test/AppNotificationTests/PackagedTests.cpp +++ b/test/AppNotificationTests/PackagedTests.cpp @@ -78,6 +78,11 @@ void PackagedTests::VerifyShowToast() BaseTestSuite::VerifyShowToast(); } +void PackagedTests::VerifyShowToastTwiceFailsWithNotificationPosted() +{ + BaseTestSuite::VerifyShowToastTwiceFailsWithNotificationPosted(); +} + void PackagedTests::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup() { BaseTestSuite::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup(); diff --git a/test/AppNotificationTests/PackagedTests.h b/test/AppNotificationTests/PackagedTests.h index 8c04e1eb89..0970d649f7 100644 --- a/test/AppNotificationTests/PackagedTests.h +++ b/test/AppNotificationTests/PackagedTests.h @@ -59,6 +59,7 @@ class PackagedTests : BaseTestSuite TEST_METHOD(VerifyToastExpiresOnReboot); TEST_METHOD(VerifyToastProgressDataSequence0Fail); TEST_METHOD(VerifyShowToast); + TEST_METHOD(VerifyShowToastTwiceFailsWithNotificationPosted); TEST_METHOD(VerifyUpdateToastProgressDataUsingValidTagAndValidGroup); TEST_METHOD(VerifyUpdateToastProgressDataUsingValidTagAndEmptyGroup); TEST_METHOD(VerifyUpdateToastProgressDataUsingEmptyTagAndEmptyGroup); diff --git a/test/AppNotificationTests/UnpackagedTests.cpp b/test/AppNotificationTests/UnpackagedTests.cpp index 100fe556ae..f4ed888054 100644 --- a/test/AppNotificationTests/UnpackagedTests.cpp +++ b/test/AppNotificationTests/UnpackagedTests.cpp @@ -78,6 +78,11 @@ void UnpackagedTests::VerifyShowToast() BaseTestSuite::VerifyShowToast(); } +void UnpackagedTests::VerifyShowToastTwiceFailsWithNotificationPosted() +{ + BaseTestSuite::VerifyShowToastTwiceFailsWithNotificationPosted(); +} + void UnpackagedTests::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup() { BaseTestSuite::VerifyUpdateToastProgressDataUsingValidTagAndValidGroup(); diff --git a/test/AppNotificationTests/UnpackagedTests.h b/test/AppNotificationTests/UnpackagedTests.h index 12faa3dcc1..78d6bd1688 100644 --- a/test/AppNotificationTests/UnpackagedTests.h +++ b/test/AppNotificationTests/UnpackagedTests.h @@ -57,6 +57,7 @@ class UnpackagedTests : BaseTestSuite TEST_METHOD(VerifyToastExpiresOnReboot); TEST_METHOD(VerifyToastProgressDataSequence0Fail); TEST_METHOD(VerifyShowToast); + TEST_METHOD(VerifyShowToastTwiceFailsWithNotificationPosted); TEST_METHOD(VerifyUpdateToastProgressDataUsingValidTagAndValidGroup); TEST_METHOD(VerifyUpdateToastProgressDataUsingValidTagAndEmptyGroup); TEST_METHOD(VerifyUpdateToastProgressDataUsingEmptyTagAndEmptyGroup); diff --git a/test/Compatibility/CompatibilityTests/CompatibilityTests.cpp b/test/Compatibility/CompatibilityTests/CompatibilityTests.cpp index 0459e51f4e..b8bc7a148d 100644 --- a/test/Compatibility/CompatibilityTests/CompatibilityTests.cpp +++ b/test/Compatibility/CompatibilityTests/CompatibilityTests.cpp @@ -242,5 +242,32 @@ namespace Test::CompatibilityTests VERIFY_IS_FALSE((WinAppSdk::Containment::IsChangeEnabled<55555>())); // explicit only VERIFY_IS_TRUE((WinAppSdk::Containment::IsChangeEnabled<99999>())); // neither } + + // Bug 61688595 gates the AppNotificationManager::Show and + // AppInstance::GetInstances noexcept-worker refactor. The change ID is declared + // locally at those callsites and is deliberately not part of the catalog, so it + // is enabled by default and only an app's explicit DisabledChanges list turns it + // off - which restores the pre-61688595 throwing hot paths. + static constexpr UINT32 c_changeId_NoexceptHotPaths{ 61688595 }; + + TEST_METHOD(VerifyNoexceptHotPathsChangeEnabledByDefault) + { + winrt::Microsoft::Windows::ApplicationModel::WindowsAppRuntime::RuntimeCompatibilityOptions options; + options.Apply(); + + VERIFY_IS_TRUE((WinAppSdk::Containment::IsChangeEnabled())); + } + + TEST_METHOD(VerifyNoexceptHotPathsChangeCanBeDisabled) + { + winrt::Microsoft::Windows::ApplicationModel::WindowsAppRuntime::RuntimeCompatibilityOptions options; + options.DisabledChanges().Append((WAR::RuntimeCompatibilityChange)c_changeId_NoexceptHotPaths); + options.Apply(); + + VERIFY_IS_FALSE((WinAppSdk::Containment::IsChangeEnabled())); + + // A neighbouring ID must be unaffected. + VERIFY_IS_TRUE((WinAppSdk::Containment::IsChangeEnabled())); + } }; }