From 0b17b955e4aeb0845d3d69a985ed5cea02b425f1 Mon Sep 17 00:00:00 2001 From: "Chris Wall (WIN SDE)" Date: Fri, 18 Sep 2026 11:47:49 -0600 Subject: [PATCH 1/2] [2.0-stable] Cherry-pick: AppNotificationManager::Show and AppInstance::GetInstances noexcept workers (#6714) Cherry-pick of #6714 (Bug 61688595) onto release/2.0-stable, gated behind contained change 61688595 so the pre-change behavior is preserved when an app disables it via RuntimeCompatibilityOptions.DisabledChanges. AppNotificationManager::Show: the throwing body moves to ShowLegacy and is mirrored by a noexcept HRESULT worker (ShowImpl) that uses RETURN_HR_IF / RETURN_IF_FAILED / CATCH_RETURN. The public Show keeps the IsSupported() early-return, then either calls ShowLegacy (change disabled) or rethrows the worker's failure HRESULT via THROW_IF_FAILED(ShowImpl(...)) (change enabled). AppLifecycle::AppInstance::GetInstances: the body moves to GetInstancesWorker, wrapped by GetInstancesImpl(IVector&) noexcept. The public GetInstances() calls the worker directly when the change is disabled, and THROW_IF_FAILED(GetInstancesImpl(...)) when it is enabled. The existing 63876312 process-handle containment inside the worker is unchanged. This keeps a failed HRESULT off the deep C++/WinRT projection rethrow path that exhausted the stack via WIL's FormatMessage-based exception logging chain. Tests: - CompatibilityTests: VerifyNoexceptHotPathsChangeEnabledByDefault and VerifyNoexceptHotPathsChangeCanBeDisabled cover the gate plumbing. - AppLifecycle FunctionalTests: GetInstancesIncludesCurrentInstance covers the refactored GetInstances boundary. - AppNotificationTests: VerifyShowToastTwiceFailsWithNotificationPosted covers Show still surfacing WPN_E_NOTIFICATION_POSTED. AB#61688595 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- dev/AppLifecycle/AppInstance.cpp | 31 ++++++++- dev/AppLifecycle/AppInstance.h | 10 +++ .../AppNotificationManager.cpp | 63 +++++++++++++++++-- dev/AppNotifications/AppNotificationManager.h | 10 +++ test/AppLifecycle/FunctionalTests.cpp | 25 ++++++++ test/AppNotificationTests/BaseTestSuite.cpp | 14 +++++ test/AppNotificationTests/BaseTestSuite.h | 1 + test/AppNotificationTests/PackagedTests.cpp | 5 ++ test/AppNotificationTests/PackagedTests.h | 1 + test/AppNotificationTests/UnpackagedTests.cpp | 5 ++ test/AppNotificationTests/UnpackagedTests.h | 1 + .../CompatibilityTests/CompatibilityTests.cpp | 27 ++++++++ 12 files changed, 186 insertions(+), 7 deletions(-) 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/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())); + } }; } From d3c56ee80f6965b46b8111c6f3b82c5863f3fb74 Mon Sep 17 00:00:00 2001 From: "Chris Wall (WIN SDE)" Date: Fri, 18 Sep 2026 15:25:57 -0600 Subject: [PATCH 2/2] Fix AppLifecycle test crash on RS5/Server 2019: colocate FrameworkUdk GetInstancesIncludesCurrentInstance crashed the TAEF test host with 0xC06D007E (delay-load module not found) on x86 Server2019 and amd64 RS5. It is the first in-process AppLifecycle test to reach a WinAppSdk::Containment::IsChangeEnabled callsite, which delay-loads Microsoft.Internal.FrameworkUdk.dll. The delay-load resolver searches the app directory (TE.ProcessHost.exe's directory == the test output folder), and AppLifecycle.vcxproj was not copying that DLL there. On newer Windows the framework package graph resolved it anyway, which is why the failure only reproduced on the 1809-based legs. Copy Microsoft.Internal.FrameworkUdk.dll next to the test binary, matching the existing pattern in AppNotificationTests, LRPTests, EnvironmentManagerTests, CompatibilityTests and others. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 57737c8b-378d-4305-9111-cdf63ec2502f --- test/AppLifecycle/AppLifecycle.vcxproj | 1 + 1 file changed, 1 insertion(+) 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 @@ +