microsoft / microsoft/WindowsAppSDK

Failure to remove Microsoft.Graphics.Display.DisplayInformation instance handlers causes crash if it is destroyed on a background thread

Open
#6,298 5 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

area-Graphics
Dominant language
C++
Stars
4.7k
Forks
471
Avg merge
3d 13h
Merged PRs (30d)
28

Description

Describe the bug

In a C# app, If I get a Microsoft.Graphics.Display.DisplayInformation instance for a window, then add an AdvancedColorInfoChanged handler and forget to remove the handler and don't explicitly dispose the instance, and the instance goes out of scope, then at some random point in the future the app crashes with an access violation exception with a native stack trace with very little information. Whilst this may be considered user error for not either disposing or removing the handler, I consider it a bug in the DisplayInformation implementation because it is so exceptionally difficult to debug. The crash only manifests at some point after the finalizer has run, and obviously this is extremely unpredictable, and the lack of information in the stack trace makes it extremely difficult to diagnose the cause. I did some more digging and have found that in fact the issue only occurs if the DisplayInformation instance is destroyed not on the UI thread (which of course will always happen in C# if dispose is not called explicitly). Moreover it seems to take an additional window event like paint or resize before the crash happens.

Even if this is deemed not a bug I am submitting it here in case others get the same crash. The crash has the following characteristics. Near the end of the stack trace: Microsoft_Graphics_Display!WindowListenerFeature::s_FeatureProc. Exception code 0xc0000005 (access violation).

Steps to reproduce the bug

In a C# app, click a button with the following code behind then try to resize the window or move the mouse around or something.

        private void test_Click(object sender, RoutedEventArgs e) {
            var displayInformation = Microsoft.Graphics.Display.DisplayInformation.CreateForWindowId(this.XamlRoot.ContentIslandEnvironment.AppWindowId);
            displayInformation.AdvancedColorInfoChanged += DisplayInformation_AdvancedColorInfoChanged;
            displayInformation = null;
            GC.Collect();
            GC.WaitForPendingFinalizers();
            GC.Collect();
        }

        private void DisplayInformation_AdvancedColorInfoChanged(Microsoft.Graphics.Display.DisplayInformation sender, object args) {
        }

Alternatively do this in a C++ app (and also then try to resize the window or move the mouse around or something).

winrt:: fire_and_forget Class::Test(Microsoft::UI::WindowId const& windowId) {
        auto di = Microsoft::Graphics::Display::DisplayInformation::CreateForWindowId(windowId);
        di.AdvancedColorInfoChanged(&Class::DisplayInformation_AdvancedColorInfoChanged);
        co_await resume_background();
    }

    void Class::DisplayInformation_AdvancedColorInfoChanged(Microsoft::Graphics::Display::DisplayInformation const& sender, Windows::Foundation::IInspectable args) {}
Expected behavior

No response

Screenshots

No response

NuGet package version

Windows App SDK 1.8.5: 1.8.260209005

Packaging type

Packaged (MSIX)

Windows version

Windows 11 version 24H2 LTSC (26100, June Update)

IDE

No response

Additional context

No response

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reproducing the crash with the C# and C++ snippets in a Windows App SDK desktop app, then trace the DisplayInformation instance and AdvancedColorInfoChanged handler through finalization on a background thread. Use the native stack ending at WindowListenerFeature::s_FeatureProc to identify the lifetime failure. Done means the repro no longer crashes after finalization and later window events, with handler cleanup still safe.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, csharp
Domain
desktop, operating-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.