Skip to content

Commit

Permalink
Leak the window when it closes on Windows 10 (#15397)
Browse files Browse the repository at this point in the history
Re: #15384

Basically, when we close a `DesktopWindowXamlSource`, it calls to `Windows_UI_Xaml!DirectUI::MetadataAPI::Reset`, which resets the XAML metadata provider _for the process_. So, closing one DWXS on one thread will force an A/V next time another thread tries to do something like... display a tooltip. Not immediately, but surely soon enough.

This was fixed in Windows 11 by os.2020!5837001. That wasn't backported to Windows 10.

This will cause a ~15MB memory leak PER WINDOW. OBVIOUSLY, this is bad, but it's less bad than crashing.

We're gonna keep using #15384 for other ideas here too.

(cherry picked from commit c589784)
Service-Card-Id: 89283538
Service-Version: 1.18
  • Loading branch information
zadjii-msft authored and DHowett committed May 23, 2023
1 parent a5c351c commit 0681596
Showing 1 changed file with 40 additions and 0 deletions.
40 changes: 40 additions & 0 deletions src/cascadia/WindowsTerminal/IslandWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,46 @@ IslandWindow::~IslandWindow()

void IslandWindow::Close()
{
static const bool isWindows11 = []() {
OSVERSIONINFOEXW osver{};
osver.dwOSVersionInfoSize = sizeof(osver);
osver.dwBuildNumber = 22000;

DWORDLONG dwlConditionMask = 0;
VER_SET_CONDITION(dwlConditionMask, VER_BUILDNUMBER, VER_GREATER_EQUAL);

if (VerifyVersionInfoW(&osver, VER_BUILDNUMBER, dwlConditionMask) != FALSE)
{
return true;
}
return false;
}();

if (!isWindows11)
{
// BODGY
// ____ ____ _____ _______ __
// | _ \ / __ \| __ \ / ____\ \ / /
// | |_) | | | | | | | | __ \ \_/ /
// | _ <| | | | | | | | |_ | \ /
// | |_) | |__| | |__| | |__| | | |
// |____/ \____/|_____/ \_____| |_|
//
// There's a bug in Windows 10 where closing a DesktopWindowXamlSource
// on any thread will free an internal static resource that's used by
// XAML for the entire process. This would result in closing window
// essentially causing the entire app to crash.
//
// To avoid this, leak the XAML island. We only need to leak this on
// Windows 10, since the bug is fixed in Windows 11.
//
// See GH #15384, MSFT:32109540
auto a{ _source };
winrt::detach_abi(_source);

// </BODGY>
}

if (_source)
{
_source.Close();
Expand Down

1 comment on commit 0681596

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@check-spelling-bot Report

🔴 Please review

See the 📜action log for details.

Unrecognized words (1)

HAX

Previously acknowledged words that are now absent Hirots NULs spand xwwyzz xxyyzz :arrow_right:
To accept ✔️ these unrecognized words as correct and remove the previously acknowledged and now absent words, run the following commands

... in a clone of the [email protected]:microsoft/terminal.git repository
on the release-1.18 branch (ℹ️ how do I use this?):

curl -s -S -L 'https://raw.githubusercontent.com/check-spelling/check-spelling/v0.0.21/apply.pl' |
perl - 'https://github.com/microsoft/terminal/actions/runs/5051730507/attempts/1'
✏️ Contributor please read this

By default the command suggestion will generate a file named based on your commit. That's generally ok as long as you add the file to your commit. Someone can reorganize it later.

⚠️ The command is written for posix shells. If it doesn't work for you, you can manually add (one word per line) / remove items to expect.txt and the excludes.txt files.

If the listed items are:

  • ... misspelled, then please correct them instead of using the command.
  • ... names, please add them to .github/actions/spelling/allow/names.txt.
  • ... APIs, you can add them to a file in .github/actions/spelling/allow/.
  • ... just things you're using, please add them to an appropriate file in .github/actions/spelling/expect/.
  • ... tokens you only need in one place and shouldn't generally be used, you can add an item in an appropriate file in .github/actions/spelling/patterns/.

See the README.md in each directory for more information.

🔬 You can test your commits without appending to a PR by creating a new branch with that extra change and pushing it to your fork. The check-spelling action will run in response to your push -- it doesn't require an open pull request. By using such a branch, you can limit the number of typos your peers see you make. 😉

If the flagged items are 🤯 false positives

If items relate to a ...

  • binary file (or some other file you wouldn't want to check at all).

    Please add a file path to the excludes.txt file matching the containing file.

    File paths are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your files.

    ^ refers to the file's path from the root of the repository, so ^README\.md$ would exclude README.md (on whichever branch you're using).

  • well-formed pattern.

    If you can write a pattern that would match it,
    try adding it to the patterns.txt file.

    Patterns are Perl 5 Regular Expressions - you can test yours before committing to verify it will match your lines.

    Note that patterns can't match multiline strings.

Please sign in to comment.