Repository: microsoft/terminal (language: cpp)

Issue:
D2D DEBUG ERROR - Memory leaks detected
### Windows Terminal version

main 10c6206

### Windows build number

10.0.22621.1555

### Other Software

_No response_

### Steps to reproduce

1. Open the terminal
2. Close the terminal

### Expected Behavior

No debug assertions get hit.

### Actual Behavior

A debug assertion is hit - stating that there are memory leaks.

Debug Output:
```
An interface [000001A8656D9E28] was created but not released. Use 'dps 000001A8656D9DC8' to view its allocation stack.
Object type: ID2D1SolidColorBrush
	Color: r=1.00 g=1.00 b=1.00 a=1.00
	Outstanding reference count: 1

An interface [000001A8656D9B88] was created but not released. Use 'dps 000001A8656D9B28' to view its allocation stack.
Object type: ID2D1SolidColorBrush
	Color: r=1.00 g=1.00 b=1.00 a=1.00
	Outstanding reference count: 1

An interface [000001A865614C48] was created but not released. Use 'dps 000001A865614BE8' to view its allocation stack.
Object type: ID2D1DeviceContext
	Outstanding reference count: 1

An interface [000001A8656C4108] was created but not released. Use 'dps 000001A8656C40A8' to view its allocation stack.
Object type: ID2D1Factory
	Factory type: D2D1_FACTORY_TYPE_SINGLE_THREADED
	Outstanding reference count: 4

D2D DEBUG ERROR - Memory leaks detected.
(66cc.4b0): Break instruction exception - code 80000003 (first chance)
KERNELBASE!wil::details::DebugBreak+0x2:
00007ffa`470a7b02 cc              int     3
```

Allocation stack 1:
```
0:000> dps 000001A8656D9DC8
000001a8`656d9dc8  00007ff9`19f7b72d D2D1Debug3!DebugRenderTargetGenerated<ID2D1BitmapRenderTarget>::CreateSolidColorBrush+0xfd
000001a8`656d9dd0  00007ff9`0a56b1a6*** WARNING: Unable to verify checksum for F:\windows-terminal\src\cascadia\CascadiaPackage\bin\x64\Debug\AppX\Microsoft.Terminal.Control.dll
 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::_resizeGlyphAtlas+0x856 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 778]
000001a8`656d9dd8  00007ff9`0a56a754 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::_resetGlyphAtlas+0x264 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 707]
000001a8`656d9de0  00007ff9`0a56c072 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::_drawText+0x72 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 943]
000001a8`656d9de8  00007ff9`0a567252 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::Render+0x172 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 226]
000001a8`656d9df0  00007ff9`0a55599f Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::AtlasEngine::Present+0x1cf [F:\windows-terminal\src\renderer\atlas\AtlasEngine.r.cpp @ 56]
000001a8`656d9df8  00007ff9`0a09f4dd Microsoft_Terminal_Control!Microsoft::Console::Render::Renderer::_PaintFrameForEngine+0x72d [F:\windows-terminal\src\renderer\base\renderer.cpp @ 179]
000001a8`656d9e00  00007ff9`0a09c8c6 Microsoft_Terminal_Control!Microsoft::Console::Render::Renderer::PaintFrame+0xc6 [F:\windows-terminal\src\renderer\base\renderer.cpp @ 77]
000001a8`656d9e08  00007ff9`0a0ad02b Microsoft_Terminal_Control!Microsoft::Console::Render::RenderThread::_ThreadProc+0x11b [F:\windows-terminal\src\renderer\base\thread.cpp @ 214]
000001a8`656d9e10  00007ff9`0a0acef2 Microsoft_Terminal_Control!Microsoft::Console::Render::RenderThread::s_ThreadProc+0x32 [F:\windows-terminal\src\renderer\base\thread.cpp @ 160]
000001a8`656d9e18  00007ffa`487c269d KERNEL32!BaseThreadInitThunk+0x1d
000001a8`656d9e20  00007ffa`4958a9f8 ntdll!RtlUserThreadStart+0x28
000001a8`656d9e28  00007ff9`19fbf5b0 D2D1Debug3!DebugComObject<DebugSolidColorBrushGenerated<ID2D1SolidColorBrush> >::`vftable'
000001a8`656d9e30  00000000`00000001
000001a8`656d9e38  00080000`00000000
000001a8`656d9e40  000001a8`656ef1e0
```

Which is: https://github.com/microsoft/terminal/blob/10c6206bfef7d045347327687266832931f47eb6/src/renderer/atlas/BackendD3D.cpp#L778

Allocation stack 2:
```
0:000> dps 000001A8656D9B28
000001a8`656d9b28  00007ff9`19f7b72d D2D1Debug3!DebugRenderTargetGenerated<ID2D1BitmapRenderTarget>::CreateSolidColorBrush+0xfd
000001a8`656d9b30  00007ff9`0a56b0e4 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::_resizeGlyphAtlas+0x794 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 777]
000001a8`656d9b38  00007ff9`0a56a754 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::_resetGlyphAtlas+0x264 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 707]
000001a8`656d9b40  00007ff9`0a56c072 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::_drawText+0x72 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 943]
000001a8`656d9b48  00007ff9`0a567252 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::Render+0x172 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 226]
000001a8`656d9b50  00007ff9`0a55599f Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::AtlasEngine::Present+0x1cf [F:\windows-terminal\src\renderer\atlas\AtlasEngine.r.cpp @ 56]
000001a8`656d9b58  00007ff9`0a09f4dd Microsoft_Terminal_Control!Microsoft::Console::Render::Renderer::_PaintFrameForEngine+0x72d [F:\windows-terminal\src\renderer\base\renderer.cpp @ 179]
000001a8`656d9b60  00007ff9`0a09c8c6 Microsoft_Terminal_Control!Microsoft::Console::Render::Renderer::PaintFrame+0xc6 [F:\windows-terminal\src\renderer\base\renderer.cpp @ 77]
000001a8`656d9b68  00007ff9`0a0ad02b Microsoft_Terminal_Control!Microsoft::Console::Render::RenderThread::_ThreadProc+0x11b [F:\windows-terminal\src\renderer\base\thread.cpp @ 214]
000001a8`656d9b70  00007ff9`0a0acef2 Microsoft_Terminal_Control!Microsoft::Console::Render::RenderThread::s_ThreadProc+0x32 [F:\windows-terminal\src\renderer\base\thread.cpp @ 160]
000001a8`656d9b78  00007ffa`487c269d KERNEL32!BaseThreadInitThunk+0x1d
000001a8`656d9b80  00007ffa`4958a9f8 ntdll!RtlUserThreadStart+0x28
000001a8`656d9b88  00007ff9`19fbf5b0 D2D1Debug3!DebugComObject<DebugSolidColorBrushGenerated<ID2D1SolidColorBrush> >::`vftable'
000001a8`656d9b90  00000000`00000001
000001a8`656d9b98  00080000`00000000
000001a8`656d9ba0  000001a8`6568ff50
```

Which is: https://github.com/microsoft/terminal/blob/10c6206bfef7d045347327687266832931f47eb6/src/renderer/atlas/BackendD3D.cpp#L777

Allocation stack 3:
```
0:000> dps 000001A865614BE8
000001a8`65614be8  00007ff9`19f5a289 D2D1Debug3!DebugFactoryGenerated<ID2D1Factory1>::CreateDxgiSurfaceRenderTarget+0x259
000001a8`65614bf0  00007ff9`0a56acdb Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::_resizeGlyphAtlas+0x38b [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 749]
000001a8`65614bf8  00007ff9`0a56a754 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::_resetGlyphAtlas+0x264 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 707]
000001a8`65614c00  00007ff9`0a56c072 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::_drawText+0x72 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 943]
000001a8`65614c08  00007ff9`0a567252 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::BackendD3D::Render+0x172 [F:\windows-terminal\src\renderer\atlas\BackendD3D.cpp @ 226]
000001a8`65614c10  00007ff9`0a55599f Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::AtlasEngine::Present+0x1cf [F:\windows-terminal\src\renderer\atlas\AtlasEngine.r.cpp @ 56]
000001a8`65614c18  00007ff9`0a09f4dd Microsoft_Terminal_Control!Microsoft::Console::Render::Renderer::_PaintFrameForEngine+0x72d [F:\windows-terminal\src\renderer\base\renderer.cpp @ 179]
000001a8`65614c20  00007ff9`0a09c8c6 Microsoft_Terminal_Control!Microsoft::Console::Render::Renderer::PaintFrame+0xc6 [F:\windows-terminal\src\renderer\base\renderer.cpp @ 77]
000001a8`65614c28  00007ff9`0a0ad02b Microsoft_Terminal_Control!Microsoft::Console::Render::RenderThread::_ThreadProc+0x11b [F:\windows-terminal\src\renderer\base\thread.cpp @ 214]
000001a8`65614c30  00007ff9`0a0acef2 Microsoft_Terminal_Control!Microsoft::Console::Render::RenderThread::s_ThreadProc+0x32 [F:\windows-terminal\src\renderer\base\thread.cpp @ 160]
000001a8`65614c38  00007ffa`487c269d KERNEL32!BaseThreadInitThunk+0x1d
000001a8`65614c40  00007ffa`4958a9f8 ntdll!RtlUserThreadStart+0x28
000001a8`65614c48  00007ff9`19fc1000 D2D1Debug3!DebugComObject<DebugDeviceContext6Generated<ID2D1DeviceContext6> >::`vftable'
000001a8`65614c50  00000000`00000000
000001a8`65614c58  0000ffff`00000000
000001a8`65614c60  00000000`00000002
```

Which is: https://github.com/microsoft/terminal/blob/10c6206bfef7d045347327687266832931f47eb6/src/renderer/atlas/BackendD3D.cpp#L749

Allocation stack 4:
```
0:000> dps 000001A8656C40A8
000001a8`656c40a8  00007ffa`42f3fef0 d2d1!D2D1CreateFactoryImpl+0xaf280
000001a8`656c40b0  00007ffa`42e91469 d2d1!D2D1CreateFactory+0xa9
000001a8`656c40b8  00007ff9`0a520cd1 Microsoft_Terminal_Control!Microsoft::Console::Render::Atlas::AtlasEngine::AtlasEngine+0xd1 [F:\windows-terminal\src\renderer\atlas\AtlasEngine.cpp @ 34]
000001a8`656c40c0  00007ff9`0a222c3e Microsoft_Terminal_Control!std::make_unique<Microsoft::Console::Render::Atlas::AtlasEngine,0>+0x4e [D:\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.35.32215\include\memory @ 3393]
000001a8`656c40c8  00007ff9`0a1d6f7f Microsoft_Terminal_Control!winrt::Microsoft::Terminal::Control::implementation::ControlCore::Initialize+0x1ef [F:\windows-terminal\src\cascadia\TerminalControl\ControlCore.cpp @ 336]
000001a8`656c40d0  00007ff9`0a27dca7 Microsoft_Terminal_Control!winrt::impl::produce<winrt::Microsoft::Terminal::Control::implementation::ControlCore,winrt::Microsoft::Terminal::Control::IControlCore>::Initialize+0xa7 [F:\windows-terminal\src\cascadia\TerminalControl\Generated Files\winrt\Microsoft.Terminal.Control.h @ 2230]
000001a8`656c40d8  00007ff9`0a3f83f5 Microsoft_Terminal_Control!winrt::impl::consume_Microsoft_Terminal_Control_IControlCore<winrt::Microsoft::Terminal::Control::IControlCore>::Initialize+0x105 [F:\windows-terminal\src\cascadia\TerminalControl\Generated Files\winrt\Microsoft.Terminal.Control.h @ 91]
000001a8`656c40e0  00007ff9`0a3724dd Microsoft_Terminal_Control!winrt::Microsoft::Terminal::Control::implementation::TermControl::_InitializeTerminal+0x22d [F:\windows-terminal\src\cascadia\TerminalControl\TermControl.cpp @ 956]
000001a8`656c40e8  00007ff9`0a39bc89 Microsoft_Terminal_Control!`winrt::Microsoft::Terminal::Control::implementation::TermControl::TermControl'::`2'::<lambda_1>::operator()<winrt::Windows::Foundation::IInspectable,winrt::Windows::Foundation::IInspectable>+0x39 [F:\windows-terminal\src\cascadia\TerminalControl\TermControl.cpp @ 118]
000001a8`656c40f0  00007ff9`0a37a79c Microsoft_Terminal_Control!winrt::impl::delegate<winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>,`winrt::Microsoft::Terminal::Control::implementation::TermControl::TermControl'::`2'::<lambda_1> >::Invoke+0x9c [F:\windows-terminal\src\cascadia\TerminalControl\Generated Files\winrt\Windows.Foundation.h @ 884]
000001a8`656c40f8  00007ffa`020dabad Windows_UI_Xaml!DirectUI::CEventSourceBase<DirectUI::IUntypedEventSource,Windows::Foundation::IEventHandler<IInspectable *>,IInspectable,IInspectable>::Raise+0x85 [onecoreuap\windows\dxaml\xcp\dxaml\lib\JoltClasses.h @ 275]
000001a8`656c4100  00007ffa`020daa93 Windows_UI_Xaml!DirectUI::CEventSourceBase<DirectUI::IUntypedEventSource,Windows::Foundation::IEventHandler<IInspectable *>,IInspectable,IInspectable>::UntypedRaise+0x43 [onecoreuap\windows\dxaml\xcp\dxaml\lib\JoltClasses.h @ 248]
000001a8`656c4108  00007ff9`19fb7c48 D2D1Debug3!DebugComObject<DebugFactory7Generated<ID2D1Factory7> >::`vftable'
000001a8`656c4110  00000000`00000001
000001a8`656c4118  00080000`00000000
000001a8`656c4120  f026b5be`dd1e28c9
```

Which is: https://github.com/microsoft/terminal/blob/10c6206bfef7d045347327687266832931f47eb6/src/renderer/atlas/AtlasEngine.cpp#L34


Discussion / hints:
This happens because this code: https://github.com/microsoft/terminal/blob/10c6206bfef7d045347327687266832931f47eb6/src/cascadia/WindowsTerminal/WindowThread.cpp#L43-L61

is called _after_ we mark the last window as having exited here:
https://github.com/microsoft/terminal/blob/10c6206bfef7d045347327687266832931f47eb6/src/cascadia/WindowsTerminal/WindowEmperor.cpp#L159-L167

This is problematic because once the last window is marked as having exited, we'll send a `WM_QUIT` message to the main thread, which will make Windows Terminal exit sometime soon, potentially before the window thread has actually exited.

But I'm preparing a PR to fix that, and it doesn't actually 100% fix the issue. The `_host = nullptr;` assignment in `RundownForExit()` implicitly destroys the XAML root here, which will make it shut down:
https://github.com/microsoft/terminal/blob/10c6206bfef7d045347327687266832931f47eb6/src/cascadia/WindowsTerminal/IslandWindow.cpp#L40

(`_source` is a `DesktopWindowXamlSource`.) But because WinUI/XAML is a very distinguished system, destroying the `DesktopWindowXamlSource` doesn't actually free any of the XAML elements, like the `TerminalPage`, which is the thing that owns the renderer instance. XAML defers destruction of its objects to some later time (the next render pass?), which only works when the event loop is actually still running. While `RundownForExit()` does continue running the event loop until all messages have been processed, there's absolutely no guarantee that it actually processed _all_ asynchronous messages. After all, some async messages could come in at some point in the far future.
So, sometimes randomly this will still result in none of the XAML objects being released and we'll leak almost all of the window's memory. This is pretty catastrophic, because a) there's no proper fix as far as I'm aware (but if you have more "creative" ideas how we can pump the loop until everything's done, I'd be happy to try it out) and b) Explorer's new tab tearout architecture unknowingly suffers from the exact same bug.
@lhecker Is this that destruction order thing you were talking about during team sync yesterday? Should I put this in 1.18?

Fix patch:
diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.cpp b/src/cascadia/WindowsTerminal/WindowEmperor.cpp
index e847547c01e..100367e9eff 100644
--- a/src/cascadia/WindowsTerminal/WindowEmperor.cpp
+++ b/src/cascadia/WindowsTerminal/WindowEmperor.cpp
@@ -140,10 +140,16 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs&
     std::thread t([weakThis, window]() {
         try
         {
-            auto cleanup = wil::scope_exit([&]() {
+            const auto decrementWindowCount = wil::scope_exit([&]() {
                 if (auto self{ weakThis.lock() })
                 {
-                    self->_windowExitedHandler(window->Peasant().GetID());
+                    self->_decrementWindowCount();
+                }
+            });
+            auto removeWindow = wil::scope_exit([&]() {
+                if (auto self{ weakThis.lock() })
+                {
+                    self->_removeWindow(window->PeasantID());
                 }
             });
 
@@ -160,7 +166,7 @@ void WindowEmperor::_createNewWindowThread(const Remoting::WindowRequestedArgs&
             // remove the window from our list of windows, before we release the
             // AppHost (and subsequently, the host's Logic() member that we use
             // elsewhere).
-            cleanup.reset();
+            removeWindow.reset();
 
             // Now that we no longer care about this thread's window, let it
             // release it's app host and flush the rest of the XAML queue.
@@ -204,17 +210,20 @@ void WindowEmperor::_windowStartedHandlerPostXAML(const std::shared_ptr<WindowTh
         lockedWindows->push_back(sender);
     }
 }
-void WindowEmperor::_windowExitedHandler(uint64_t senderID)
+
+void WindowEmperor::_removeWindow(uint64_t senderID)
 {
     auto lockedWindows{ _windows.lock() };
 
     // find the window in _windows who's peasant's Id matches the peasant's Id
     // and remove it
-    std::erase_if(*lockedWindows,
-                  [&](const auto& w) {
-                      return w->Peasant().GetID() == senderID;
-                  });
+    std::erase_if(*lockedWindows, [&](const auto& w) {
+        return w->PeasantID() == senderID;
+    });
+}
 
+void WindowEmperor::_decrementWindowCount()
+{
     // When we run out of windows, exit our process if and only if:
     // * We're not allowed to run headless OR
     // * we've explicitly been told to "quit", which should fully exit the Terminal.
@@ -226,6 +235,7 @@ void WindowEmperor::_windowExitedHandler(uint64_t senderID)
         _close();
     }
 }
+
 // Method Description:
 // - Set up all sorts of handlers now that we've determined that we're a process
 //   that will end up hosting the windows. These include:
diff --git a/src/cascadia/WindowsTerminal/WindowEmperor.h b/src/cascadia/WindowsTerminal/WindowEmperor.h
index 200546c21ba..96688ee08c2 100644
--- a/src/cascadia/WindowsTerminal/WindowEmperor.h
+++ b/src/cascadia/WindowsTerminal/WindowEmperor.h
@@ -55,7 +55,8 @@ class WindowEmperor : public std::enable_shared_from_this<WindowEmperor>
     bool _quitting{ false };
 
     void _windowStartedHandlerPostXAML(const std::shared_ptr<WindowThread>& sender);
-    void _windowExitedHandler(uint64_t senderID);
+    void _removeWindow(uint64_t senderID);
+    void _decrementWindowCount();
 
     void _becomeMonarch();
     void _numberOfWindowsChanged(const winrt::Windows::Foundation::IInspectable&, const winrt::Windows::Foundation::IInspectable&);
diff --git a/src/cascadia/WindowsTerminal/WindowThread.cpp b/src/cascadia/WindowsTerminal/WindowThread.cpp
index 14cafb7b79b..1076d247ced 100644
--- a/src/cascadia/WindowsTerminal/WindowThread.cpp
+++ b/src/cascadia/WindowsTerminal/WindowThread.cpp
@@ -128,7 +128,8 @@ int WindowThread::_messagePump()
     }
     return 0;
 }
-winrt::Microsoft::Terminal::Remoting::Peasant WindowThread::Peasant()
+
+uint64_t WindowThread::PeasantID()
 {
-    return _peasant;
+    return _peasant.GetID();
 }
diff --git a/src/cascadia/WindowsTerminal/WindowThread.h b/src/cascadia/WindowsTerminal/WindowThread.h
index 86b0854639e..8699a791092 100644
--- a/src/cascadia/WindowsTerminal/WindowThread.h
+++ b/src/cascadia/WindowsTerminal/WindowThread.h
@@ -17,7 +17,7 @@ class WindowThread : public std::enable_shared_from_this<WindowThread>
     int RunMessagePump();
     void RundownForExit();
 
-    winrt::Microsoft::Terminal::Remoting::Peasant Peasant();
+    uint64_t PeasantID();
 
     WINRT_CALLBACK(UpdateSettingsRequested, winrt::delegate<void()>);
 


Test patch: