Skip to content

Guard island input site fallback during shutdown - #11063

Open
Jon Wiswall (jonwis) wants to merge 1 commit into
microsoft:mainfrom
jonwis:fix/island-input-site-shutdown-null-check
Open

Guard island input site fallback during shutdown#11063
Jon Wiswall (jonwis) wants to merge 1 commit into
microsoft:mainfrom
jonwis:fix/island-input-site-shutdown-null-check

Conversation

@jonwis

Copy link
Copy Markdown
Member

Fixes a crash during shutdown when the input island site has already been destroyed, avoid calling through a nullptr.

PR Type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Description

A consistent crash on my machine in the "Tempo of UWP" app from MikeHillberg happens during process shutdown (hitting alt-f4.)

00 Microsoft_UI_Xaml!std::vector<CInputServices::IslandInputSiteRegistration,std::allocator<CInputServices::IslandInputSiteRegistration> >::empty
01 Microsoft_UI_Xaml!CInputServices::GetPrimaryRegisteredIslandInputSite
02 Microsoft_UI_Xaml!CDependencyObject::GetElementIslandInputSite
03 Microsoft_UI_Xaml!TextServicesHost::TxGetWindow
04 WinUIEdit!CTxtEdit::TxGetWindow
05 WinUIEdit!CTouchHandlerImpl::ShowGrippersHelper

In frame 2, the input services object returned was null, causing frames 1 & 0 to operate on a null this and crash.

  • Add a null check when reading through the pointers
  • Verified that all callsites have appropriate "returns null" behavior except DirectManipulation
  • This particular crash is from input services, not DManip

Current Behavior

Currently, apps crash when shutting down in this way.

New Behavior

No longer crashes.

Motivation and Context

Crashing isn't fun. Less crashes is better.

How Has This Been Tested?

  • I have performed a self-review of my own code
  • I have added tests to cover my changes

@microsoft-github-policy-service microsoft-github-policy-service Bot added the needs-triage Issue needs to be triaged by the area owners label Apr 13, 2026
@TheBlueSky

Copy link
Copy Markdown

Jon Wiswall (@jonwis) hey, any plan to pick this back up?

I hit the same crash and had a repro built before I found your PR. Just re-tested on the latest bits: still reproduces on 1.7, 1.8 and 2.4.0, and that unguarded GetInputServices()->GetPrimaryRegisteredIslandInputSite() call is still sitting in main. Looks like the diff has gone stale too, since main moved from src/dxaml/... to dxaml/....

Would you rather rebase this one and get it merged, or should I open an issue and a new PR with the same fix? Happy either way, just don't want to duplicate your work.

@jonwis

Copy link
Copy Markdown
Member Author

Jon Wiswall (Jon Wiswall (@jonwis)) hey, any plan to pick this back up?

I hit the same crash and had a repro built before I found your PR. Just re-tested on the latest bits: still reproduces on 1.7, 1.8 and 2.4.0, and that unguarded GetInputServices()->GetPrimaryRegisteredIslandInputSite() call is still sitting in main. Looks like the diff has gone stale too, since main moved from src/dxaml/... to dxaml/....

Would you rather rebase this one and get it merged, or should I open an issue and a new PR with the same fix? Happy either way, just don't want to duplicate your work.

If you have time Essam (@TheBlueSky) please feel free to make a new issue/PR to move this to completion.

Essam (TheBlueSky) added a commit to TheBlueSky/microsoft-ui-xaml that referenced this pull request Aug 26, 2026
CDependencyObject::GetElementIslandInputSite() called
CCoreServices::GetInputServices()->GetPrimaryRegisteredIslandInputSite()
without checking the result of GetInputServices(). By the time a focused
TextBox is torn down during framework shutdown the input services are
already gone, so the call runs on a null "this" and faults while reading
m_islandInputSiteRegistrations.

Null-check core services and input services and return nullptr instead.
Every caller already funnels the result through
CInputServices::GetUnderlyingInputHwndFromIslandInputSite(), which is
declared _In_opt_ and null-checks its argument, so "no island input site"
is already an expected result.

Carries forward the fix proposed by @jonwis in microsoft#11063, rebased onto the
current dxaml/ layout.
@TheBlueSky

Essam (TheBlueSky) commented Aug 26, 2026

Copy link
Copy Markdown

Jon Wiswall (@jonwis) Opened #11653 with the repro and the version details, and #11654 carries your fix rebased onto the current dxaml/ layout. You're credited in both.

Thanks for the original diagnosis — the null GetInputServices() call was the whole answer and it saved me a lot of digging.

@godlytalias

Copy link
Copy Markdown
Contributor

Essam (@TheBlueSky) Please check whether the commit, 691c6a7 fixes your issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-triage Issue needs to be triaged by the area owners

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants