Fix TerminalControl crash on exit (#10031)

## Summary of the Pull Request

ControlCore's _renderer (IRenderTarget) is allocated as std::unique_ptr,
but is given to Terminal::CreateFromSettings as a reference.
ControlCore::Close deallocates the _renderer, but if ThrottledFuncs
are still scheduled to call ControlCore::UpdatePatternLocations
it'll cause Terminal::UpdatePatterns to be called, which in turn ends up
accessing the deallocated IRenderTarget reference and lead to a crash.

A proper solution with shared pointers is nontrivial and should be
attempted at a later point in time. This solution moves the teardown of
the _renderer into ControlCore::~ControlCore, where we can be certain
that no further strong references are held by ThrottledFuncs.

## PR Checklist
* [x] Closes #9910
* [x] I work here
* [x] Tests added/passed

## Validation Steps Performed

The crash is a race condition and inherently hard to reproduce.
During validation this PR didn't appear to introduce new crashes.

.github/actions/spelling/dictionary/apis.txt

@@ -4,11 +4,13 @@ alignof
 bitfield
 bitfields
 BUILDNUMBER
+charconv
 CLASSNOTAVAILABLE
 cmdletbinding
-colspan
 COLORPROPERTY
+colspan
 COMDLG
+cstdint
 CXICON
 CYICON
 D2DERR_SHADER_COMPILE_FAILED
@@ -52,6 +54,7 @@ IFile
 IInheritable
 IMap
 IObject
+iosfwd
 IPackage
 IPeasant
 IStorage
@@ -67,7 +70,6 @@ llu
 localtime
 lround
 LSHIFT
-MULTIPLEUSE
 msappx
 MULTIPLEUSE
 NCHITTEST
@@ -91,7 +93,6 @@ PAGESCROLL
 PICKFOLDERS
 pmr
 REGCLS
-REGCLS
 RETURNCMD
 rfind
 roundf
@@ -100,23 +101,22 @@ rx
 schandle
 semver
 serializer
+SHELLEXECUTEINFOW
 shobjidl
-SINGLEUSE
 SHOWMINIMIZED
+SINGLEUSE
 SIZENS
 smoothstep
-GETDESKWALLPAPER
-SHELLEXECUTEINFOW
 snprintf
 spsc
 sregex
 STDCPP
-strchr
 STDMETHOD
+strchr
+streambuf
 Stubless
 Subheader
 Subpage
-UPDATEINIFILE
 syscall
 TBPF
 THEMECHANGED
@@ -135,12 +135,19 @@ wsregex
 wwinmain
 XDocument
 XElement
+xfacet
 xhash
+xiosbase
+xlocale
+xlocbuf
+xlocinfo
 xlocmes
 xlocmon
 xlocnum
 xloctime
+xmemory
 XParse
+xstddef
 xstring
 xtree
 xutility

.github/actions/spelling/expect/expect.txt

@@ -27,7 +27,6 @@ ADDREF
 addressof
 ADDSTRING
 ADDTOOL
-aef
 AEnd
 AFew
 AFill
@@ -98,7 +97,6 @@ ASingle
 asm
 asmv
 asmx
-aspnet
 aspx
 astextplain
 AStomps
@@ -123,7 +121,6 @@ AVerify
 AVI
 awch
 azuredevopspodcast
-azurewebsites
 azzle
 backend
 backgrounded
@@ -174,7 +171,6 @@ BODGY
 BOLDFONT
 BOOLIFY
 bools
-boostorg
 Bopomofo
 Borland
 BOTTOMLEFT
@@ -187,7 +183,6 @@ branchconfig
 BRK
 Browsable
 bsearch
-BSODs
 bstr
 BTNFACE
 buf
@@ -202,7 +197,7 @@ BValue
 byref
 bytearray
 bytebuffer
-Cac
+cac
 callee
 cang
 capslock
@@ -302,7 +297,7 @@ codepage
 codepath
 codepoint
 codeproject
-COINIT
+coinit
 COLLECTIONURI
 colorizing
 colororacle
@@ -487,7 +482,6 @@ CYSIZEFRAME
 CYSMICON
 CYVIRTUALSCREEN
 CYVSCROLL
-dahall
 dai
 DATABLOCK
 DATAVIEW
@@ -575,7 +569,6 @@ defaultsettings
 DEFAULTTONEAREST
 DEFAULTTONULL
 DEFAULTTOPRIMARY
-DEFCON
 defectdefs
 DEFERERASE
 deff
@@ -603,7 +596,6 @@ desktopwindowxamlsource
 dest
 DESTINATIONNAME
 devblogs
-developercommunity
 devicecode
 devicefamily
 devops
@@ -671,13 +663,13 @@ DUNIT
 dup'ed
 dvi
 dw
+dwl
 DWLP
 dwm
 dwmapi
 dword
 dwrite
 dwriteglyphrundescriptionclustermap
-dwl
 dxgi
 dxgidwm
 dxinterop
@@ -770,10 +762,8 @@ fclose
 fcntl
 fdc
 FDD
-fde
 fdopen
 fdw
-fea
 fesb
 FFDE
 FFrom
@@ -808,7 +798,6 @@ flyout
 fmodern
 fmtarg
 fmtid
-fmtlib
 FOLDERID
 FONTCHANGE
 fontdlg
@@ -1013,10 +1002,9 @@ horiz
 HORZ
 hostable
 hostlib
-hotkeys
 HPA
 HPAINTBUFFER
-HPCON
+hpcon
 hpj
 hpp
 HPR
@@ -1172,7 +1160,6 @@ IRenderer
 IScheme
 ISelection
 IShell
-isocpp
 issuecomment
 IState
 IStoryboard
@@ -1290,7 +1277,7 @@ listptr
 listptrsize
 lk
 lld
-llvm
+LLVM
 llx
 LMENU
 LMNOP
@@ -1435,7 +1422,6 @@ mingw
 minimizeall
 minkernel
 MINMAXINFO
-mintty
 minwin
 minwindef
 Mip
@@ -1482,7 +1468,7 @@ MSIL
 msix
 msrc
 msvcrt
-msvcrtd
+MSVCRTD
 MSVS
 msys
 msysgit
@@ -1508,7 +1494,7 @@ namestream
 nano
 natvis
 nbsp
-Nc
+nc
 NCCALCSIZE
 NCCREATE
 NCLBUTTONDOWN
@@ -1631,7 +1617,6 @@ numlock
 numpad
 NUMSCROLL
 nupkg
-NVDA
 NVIDIA
 NVR
 Nx
@@ -1662,11 +1647,11 @@ ONECOREWINDOWS
 onehalf
 ONLCR
 Oo
-openconsoleproxy
 openbash
 opencode
 opencon
 openconsole
+openconsoleproxy
 OPENIF
 OPENLINK
 openps
@@ -1784,7 +1769,6 @@ pid
 pidl
 PIDLIST
 pii
-pinam
 pinvoke
 pipename
 pipestr
@@ -1928,7 +1912,6 @@ pythonw
 qi
 QJ
 qo
-QOL
 QRSTU
 qsort
 queryable
@@ -1999,7 +1982,7 @@ REGSTR
 reingest
 Relayout
 RELBINPATH
-remoting
+Remoting
 renamer
 renderengine
 rendersize
@@ -2077,8 +2060,8 @@ runut
 runxamlformat
 rvalue
 RVERTICAL
-rxvt
 RWIN
+rxvt
 safearray
 SAFECAST
 safemath
@@ -2522,14 +2505,14 @@ unicode
 UNICODESTRING
 UNICODETEXT
 UNICRT
-UNINIT
+uninit
 uninitialize
 uninstall
 Uniscribe
 unittest
 unittesting
 universaltest
-Unk
+unk
 unknwn
 unmark
 UNORM
@@ -2537,7 +2520,6 @@ unparseable
 unpause
 Unregister
 Unregistering
-unte
 untests
 untextured
 untimes
@@ -2594,7 +2576,6 @@ Vcount
 vcpkg
 vcprintf
 vcproj
-vcrt
 vcvarsall
 vcxitems
 vcxproj
@@ -2830,7 +2811,6 @@ wx
 wxh
 xa
 xact
-xamarin
 xaml
 Xamlmeta
 xargs
@@ -2872,8 +2852,9 @@ XSubstantial
 xtended
 xterm
 XTest
-XTPUSHSGR
 XTPOPSGR
+XTPUSHSGR
+xtr
 xunit
 xutr
 xvalue

.github/actions/spelling/expect/web.txt

@@ -15,5 +15,4 @@ winui
 appshellintegration
 cppreference
 gfycat
-what3words
 Guake

.vscode/settings.json

@@ -34,7 +34,67 @@
         "xtree": "cpp",
         "xutility": "cpp",
         "span": "cpp",
-        "string_span": "cpp"
+        "string_span": "cpp",
+        "algorithm": "cpp",
+        "atomic": "cpp",
+        "bit": "cpp",
+        "cctype": "cpp",
+        "charconv": "cpp",
+        "chrono": "cpp",
+        "clocale": "cpp",
+        "cmath": "cpp",
+        "compare": "cpp",
+        "complex": "cpp",
+        "concepts": "cpp",
+        "condition_variable": "cpp",
+        "cstdarg": "cpp",
+        "cstddef": "cpp",
+        "cstdint": "cpp",
+        "cstdio": "cpp",
+        "cstdlib": "cpp",
+        "cstring": "cpp",
+        "ctime": "cpp",
+        "cwchar": "cpp",
+        "cwctype": "cpp",
+        "exception": "cpp",
+        "filesystem": "cpp",
+        "fstream": "cpp",
+        "functional": "cpp",
+        "iomanip": "cpp",
+        "ios": "cpp",
+        "iosfwd": "cpp",
+        "iostream": "cpp",
+        "iterator": "cpp",
+        "limits": "cpp",
+        "locale": "cpp",
+        "map": "cpp",
+        "memory_resource": "cpp",
+        "mutex": "cpp",
+        "new": "cpp",
+        "numeric": "cpp",
+        "optional": "cpp",
+        "ostream": "cpp",
+        "ratio": "cpp",
+        "set": "cpp",
+        "shared_mutex": "cpp",
+        "sstream": "cpp",
+        "stdexcept": "cpp",
+        "stop_token": "cpp",
+        "streambuf": "cpp",
+        "string": "cpp",
+        "system_error": "cpp",
+        "thread": "cpp",
+        "typeinfo": "cpp",
+        "unordered_map": "cpp",
+        "unordered_set": "cpp",
+        "xfacet": "cpp",
+        "xiosbase": "cpp",
+        "xlocale": "cpp",
+        "xlocbuf": "cpp",
+        "xlocinfo": "cpp",
+        "xmemory": "cpp",
+        "xstddef": "cpp",
+        "xtr1common": "cpp"
     },
     "files.exclude": {
         "**/bin/**": true,

src/cascadia/TerminalControl/ControlCore.cpp

@@ -100,6 +100,29 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     ControlCore::~ControlCore()
     {
         Close();
+
+        // Before destroying this instance we must ensure that we destroy the _renderer
+        // before the _renderEngine, as well as calling _renderer->TriggerTeardown().
+        // _renderEngine will be destroyed naturally after this ~destructor() returns.
+
+        decltype(_renderer) renderer;
+        {
+            // GH#8734:
+            // We lock the terminal here to make sure it isn't still being
+            // used in the connection thread before we destroy the renderer.
+            // However, we must unlock it again prior to triggering the
+            // teardown, to avoid the render thread being deadlocked. The
+            // renderer may be waiting to acquire the terminal lock, while
+            // we're waiting for the renderer to finish.
+            auto lock = _terminal->LockForWriting();
+
+            _renderer.swap(renderer);
+        }
+
+        if (renderer)
+        {
+            renderer->TriggerTeardown();
+        }
     }
 
     bool ControlCore::Initialize(const double actualWidth,
@@ -1164,30 +1187,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             // don't really care to wait for the connection to be completely
             // closed. We can just do it whenever.
             _asyncCloseConnection();
-
-            {
-                // GH#8734:
-                // We lock the terminal here to make sure it isn't still being
-                // used in the connection thread before we destroy the renderer.
-                // However, we must unlock it again prior to triggering the
-                // teardown, to avoid the render thread being deadlocked. The
-                // renderer may be waiting to acquire the terminal lock, while
-                // we're waiting for the renderer to finish.
-                auto lock = _terminal->LockForWriting();
-            }
-
-            if (auto localRenderEngine{ std::exchange(_renderEngine, nullptr) })
-            {
-                if (auto localRenderer{ std::exchange(_renderer, nullptr) })
-                {
-                    localRenderer->TriggerTeardown();
-                    // renderer is destroyed
-                }
-                // renderEngine is destroyed
-            }
-
-            // we don't destroy _terminal here; it now has the same lifetime as the
-            // control.
         }
     }
 

src/cascadia/UnitTests_TerminalCore/ScrollTest.cpp

@@ -45,7 +45,7 @@ namespace
         virtual void TriggerRedraw(const COORD* const){};
         virtual void TriggerRedrawCursor(const COORD* const){};
         virtual void TriggerRedrawAll(){};
-        virtual void TriggerTeardown(){};
+        virtual void TriggerTeardown() noexcept {};
         virtual void TriggerSelection(){};
         virtual void TriggerScroll(){};
         virtual void TriggerScroll(const COORD* const delta)

src/host/ScreenBufferRenderTarget.cpp

@@ -51,7 +51,7 @@ void ScreenBufferRenderTarget::TriggerRedrawAll()
     }
 }
 
-void ScreenBufferRenderTarget::TriggerTeardown()
+void ScreenBufferRenderTarget::TriggerTeardown() noexcept
 {
     auto* pRenderer = ServiceLocator::LocateGlobals().pRender;
     const auto* pActive = &ServiceLocator::LocateGlobals().getConsoleInformation().GetActiveOutputBuffer().GetActiveBuffer();

src/host/ScreenBufferRenderTarget.hpp

@@ -33,7 +33,7 @@ public:
     void TriggerRedraw(const COORD* const pcoord) override;
     void TriggerRedrawCursor(const COORD* const pcoord) override;
     void TriggerRedrawAll() override;
-    void TriggerTeardown() override;
+    void TriggerTeardown() noexcept override;
     void TriggerSelection() override;
     void TriggerScroll() override;
     void TriggerScroll(const COORD* const pcoordDelta) override;

src/renderer/base/renderer.cpp

@@ -319,7 +319,7 @@ void Renderer::TriggerRedrawAll()
 // - <none>
 // Return Value:
 // - <none>
-void Renderer::TriggerTeardown()
+void Renderer::TriggerTeardown() noexcept
 {
     // We need to shut down the paint thread on teardown.
     _pThread->WaitForPaintCompletionAndDisable(INFINITE);

src/renderer/base/renderer.hpp

@@ -52,7 +52,7 @@ namespace Microsoft::Console::Render
         void TriggerRedraw(const COORD* const pcoord) override;
         void TriggerRedrawCursor(const COORD* const pcoord) override;
         void TriggerRedrawAll() override;
-        void TriggerTeardown() override;
+        void TriggerTeardown() noexcept override;
 
         void TriggerSelection() override;
         void TriggerScroll() override;

src/renderer/inc/DummyRenderTarget.hpp

@@ -25,7 +25,7 @@ public:
     void TriggerRedraw(const COORD* const /*pcoord*/) override {}
     void TriggerRedrawCursor(const COORD* const /*pcoord*/) override {}
     void TriggerRedrawAll() override {}
-    void TriggerTeardown() override {}
+    void TriggerTeardown() noexcept override {}
     void TriggerSelection() override {}
     void TriggerScroll() override {}
     void TriggerScroll(const COORD* const /*pcoordDelta*/) override {}

src/renderer/inc/IRenderTarget.hpp

@@ -38,7 +38,7 @@ namespace Microsoft::Console::Render
         virtual void TriggerRedrawCursor(const COORD* const pcoord) = 0;
 
         virtual void TriggerRedrawAll() = 0;
-        virtual void TriggerTeardown() = 0;
+        virtual void TriggerTeardown() noexcept = 0;
 
         virtual void TriggerSelection() = 0;
         virtual void TriggerScroll() = 0;

src/renderer/inc/IRenderer.hpp

@@ -39,7 +39,7 @@ namespace Microsoft::Console::Render
         virtual void TriggerRedrawCursor(const COORD* const pcoord) = 0;
 
         virtual void TriggerRedrawAll() = 0;
-        virtual void TriggerTeardown() = 0;
+        virtual void TriggerTeardown() noexcept = 0;
 
         virtual void TriggerSelection() = 0;
         virtual void TriggerScroll() = 0;