Fix cleaning up persisted layouts after disabling them

.config/configuration.vsEnterprise.winget

@@ -1,43 +0,0 @@
-# yaml-language-server: $schema=https://aka.ms/configuration-dsc-schema/0.2
-# Reference: https://github.com/microsoft/terminal/blob/main/README.md#developer-guidance
-properties:
-  resources:
-    - resource: Microsoft.Windows.Developer/DeveloperMode
-      directives:
-        description: Enable Developer Mode
-        allowPrerelease: true
-        # Requires elevation for the set operation
-        securityContext: elevated
-      settings:
-        Ensure: Present
-    - resource: Microsoft.WinGet.DSC/WinGetPackage
-      id: powershell
-      directives:
-        description: Install PowerShell 7
-        # Requires elevation for the set operation (i.e., installation)
-        securityContext: elevated
-      settings:
-        id: Microsoft.PowerShell
-        source: winget
-    - resource: Microsoft.WinGet.DSC/WinGetPackage
-      id: vsPackage
-      directives:
-        description: Install Visual Studio 2022 Enterprise (any edition is OK)
-        # Requires elevation for the set operation (i.e., installation)
-        securityContext: elevated
-      settings:
-        id: Microsoft.VisualStudio.2022.Enterprise
-        source: winget
-    - resource: Microsoft.VisualStudio.DSC/VSComponents
-      dependsOn:
-        - vsPackage
-      directives:
-        description: Install required VS workloads from project .vsconfig file
-        allowPrerelease: true
-        # Requires elevation for the get and set operations
-        securityContext: elevated
-      settings:
-        productId: Microsoft.VisualStudio.Product.Enterprise
-        channelId: VisualStudio.17.Release
-        vsConfigFile: '${WinGetConfigRoot}\..\.vsconfig'
-  configurationVersion: 0.2.0

.config/configuration.vsProfessional.winget

@@ -1,43 +0,0 @@
-# yaml-language-server: $schema=https://aka.ms/configuration-dsc-schema/0.2
-# Reference: https://github.com/microsoft/terminal/blob/main/README.md#developer-guidance
-properties:
-  resources:
-    - resource: Microsoft.Windows.Developer/DeveloperMode
-      directives:
-        description: Enable Developer Mode
-        allowPrerelease: true
-        # Requires elevation for the set operation
-        securityContext: elevated
-      settings:
-        Ensure: Present
-    - resource: Microsoft.WinGet.DSC/WinGetPackage
-      id: powershell
-      directives:
-        description: Install PowerShell 7
-        # Requires elevation for the set operation (i.e., installation)
-        securityContext: elevated
-      settings:
-        id: Microsoft.PowerShell
-        source: winget
-    - resource: Microsoft.WinGet.DSC/WinGetPackage
-      id: vsPackage
-      directives:
-        description: Install Visual Studio 2022 Professional (any edition is OK)
-        # Requires elevation for the set operation (i.e., installation)
-        securityContext: elevated
-      settings:
-        id: Microsoft.VisualStudio.2022.Professional
-        source: winget
-    - resource: Microsoft.VisualStudio.DSC/VSComponents
-      dependsOn:
-        - vsPackage
-      directives:
-        description: Install required VS workloads from project .vsconfig file
-        allowPrerelease: true
-        # Requires elevation for the get and set operations
-        securityContext: elevated
-      settings:
-        productId: Microsoft.VisualStudio.Product.Professional
-        channelId: VisualStudio.17.Release
-        vsConfigFile: '${WinGetConfigRoot}\..\.vsconfig'
-  configurationVersion: 0.2.0

.config/configuration.winget

@@ -1,43 +0,0 @@
-# yaml-language-server: $schema=https://aka.ms/configuration-dsc-schema/0.2
-# Reference: https://github.com/microsoft/terminal/blob/main/README.md#developer-guidance
-properties:
-  resources:
-    - resource: Microsoft.Windows.Developer/DeveloperMode
-      directives:
-        description: Enable Developer Mode
-        allowPrerelease: true
-        # Requires elevation for the set operation
-        securityContext: elevated
-      settings:
-        Ensure: Present
-    - resource: Microsoft.WinGet.DSC/WinGetPackage
-      id: powershell
-      directives:
-        description: Install PowerShell 7
-        # Requires elevation for the set operation (i.e., installation)
-        securityContext: elevated
-      settings:
-        id: Microsoft.PowerShell
-        source: winget
-    - resource: Microsoft.WinGet.DSC/WinGetPackage
-      id: vsPackage
-      directives:
-        description: Install Visual Studio 2022 Community (any edition is OK)
-        # Requires elevation for the set operation (i.e., installation)
-        securityContext: elevated
-      settings:
-        id: Microsoft.VisualStudio.2022.Community
-        source: winget
-    - resource: Microsoft.VisualStudio.DSC/VSComponents
-      dependsOn:
-        - vsPackage
-      directives:
-        description: Install required VS workloads from project .vsconfig file
-        allowPrerelease: true
-        # Requires elevation for the get and set operations
-        securityContext: elevated
-      settings:
-        productId: Microsoft.VisualStudio.Product.Community
-        channelId: VisualStudio.17.Release
-        vsConfigFile: '${WinGetConfigRoot}\..\.vsconfig'
-  configurationVersion: 0.2.0

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

@@ -1945,7 +1945,6 @@ VPACKMANIFESTDIRECTORY
 VPR
 VREDRAW
 vsc
-vsconfig
 vscprintf
 VSCROLL
 vsdevshell

README.md

@@ -340,19 +340,6 @@ If you would like to ask a question that you feel doesn't warrant an issue
 
 ## Prerequisites
 
-You can configure your environment to build Terminal in one of two ways:
-
-### Using WinGet configuration file
-
-After cloning the repository, you can use a [WinGet configuration file](https://learn.microsoft.com/en-us/windows/package-manager/configuration/#use-a-winget-configuration-file-to-configure-your-machine)
-to set up your environment. The [default configuration file](.config/configuration.winget) installs Visual Studio 2022 Community & rest of the required tools. There are two other variants of the configuration file available in the [.config](.config) directory for Enterprise & Professional editions of Visual Studio 2022. To run the default configuration file, you can either double-click the file from explorer or run the following command:
-
-```powershell
-winget configure .config\configuration.winget
-```
-
-### Manual configuration
-
 * You must be running Windows 10 2004 (build >= 10.0.19041.0) or later to run
   Windows Terminal
 * You must [enable Developer Mode in the Windows Settings

build/packages.config

@@ -1,5 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <packages>
+  <package id="MUXCustomBuildTasks" version="1.0.48" targetFramework="native" />
   <package id="Microsoft.Taef" version="10.93.240607003" targetFramework="native" />
   <package id="Microsoft.Internal.PGO-Helpers.Cpp" version="0.2.34" targetFramework="native" />
   <package id="Microsoft.Debugging.Tools.PdbStr" version="20220617.1556.0" targetFramework="native" />

build/pipelines/ob-nightly.yml

@@ -25,7 +25,7 @@ variables:
 extends:
   template: templates-v2/pipeline-onebranch-full-release-build.yml
   parameters:
-    official: false
+    official: true
     branding: Canary
     buildTerminal: true
     pgoBuildMode: None # BODGY - OneBranch is on VS 17.10, which is known to be the worst
@@ -44,7 +44,6 @@ extends:
     symbolExpiryTime: 15
     symbolPublishingSubscription: $(SymbolPublishingServiceConnection)
     symbolPublishingProject: $(SymbolPublishingProject)
-    buildPlatforms: [x64]
     ${{ if eq(true, parameters.publishToAzure) }}:
       extraPublishJobs:
         - template: build/pipelines/templates-v2/job-deploy-to-azure-storage.yml@self

build/pipelines/templates-v2/job-build-project.yml

@@ -115,10 +115,6 @@ jobs:
     clean: true
     submodules: true
     persistCredentials: True
-  - template: steps-install-powershell-modules.yml
-    parameters:
-      modules: [Az.Accounts, Az.Storage, Az.Network, Az.Resources, Az.Compute]
-
 
   # This generates either nothing for BuildTargetParameter, or /t:X;Y;Z, to control targets later.
   - pwsh: |-

build/pipelines/templates-v2/job-deploy-to-azure-storage.yml

@@ -75,9 +75,10 @@ jobs:
       }
     displayName: "Wrangle Unpackaged builds into place, rename"
 
-  - template: steps-install-powershell-modules.yml
-    parameters:
-      modules: [Az.Accounts, Az.Storage, Az.Network, Az.Resources, Az.Compute]
+  - powershell: |-
+      Get-PackageProvider -Name NuGet -ForceBootstrap
+      Install-Module -Verbose -AllowClobber -Force Az.Accounts, Az.Storage, Az.Network, Az.Resources, Az.Compute
+    displayName: Install Azure Module Dependencies
 
   - task: AzureFileCopy@6
     displayName: Publish to Storage Account

build/pipelines/templates-v2/job-publish-symbols-using-symbolrequestprod-api.yml

@@ -52,9 +52,10 @@ jobs:
       itemPattern: '**/*.pdb'
       targetPath: '$(Build.SourcesDirectory)/bin'
 
-  - template: steps-install-powershell-modules.yml
-    parameters:
-      modules: [Az.Accounts, Az.Storage, Az.Network, Az.Resources, Az.Compute]
+  - powershell: |-
+      Get-PackageProvider -Name NuGet -ForceBootstrap
+      Install-Module -Verbose -AllowClobber -Force Az.Accounts, Az.Storage, Az.Network, Az.Resources, Az.Compute
+    displayName: Install Azure Module Dependencies
 
   # Transit the Azure token from the Service Connection into a secret variable for the rest of the pipeline to use.
   - task: AzurePowerShell@5

build/pipelines/templates-v2/steps-install-powershell-modules.yml

@@ -1,15 +0,0 @@
-parameters:
-  - name: modules
-    type: object
-    default: []
-
-steps:
-  - pwsh: |-
-      Register-PSResourceRepository -Name TerminalDependencies -Uri https://pkgs.dev.azure.com/shine-oss/terminal/_packaging/TerminalDependencies/nuget/v2 -Trusted -ErrorAction:Ignore
-      Install-PSResource -Repository TerminalDependencies -Name ${{ join(',',parameters.modules) }} -Debug -Verbose -Confirm:$false -AcceptLicense -TrustRepository -Reinstall
-    displayName: Install Modules for PowerShell 7+
-
-  - powershell: |-
-      Register-PSResourceRepository -Name TerminalDependencies -Uri https://pkgs.dev.azure.com/shine-oss/terminal/_packaging/TerminalDependencies/nuget/v2 -Trusted -ErrorAction:Ignore
-      Install-PSResource -Repository TerminalDependencies -Name ${{ join(',',parameters.modules) }} -Debug -Verbose -Confirm:$false -AcceptLicense -TrustRepository -Reinstall
-    displayName: Install Modules for PowerShell 5.1

dep/vcpkg-overlay-ports/fmt/fix-write-batch.patch

@@ -1,13 +0,0 @@
-diff --git a/CMakeLists.txt b/CMakeLists.txt
-index 88c12148..967b53dd 100644
---- a/CMakeLists.txt
-+++ b/CMakeLists.txt
-@@ -260,7 +260,7 @@ if (FMT_MASTER_PROJECT AND CMAKE_GENERATOR MATCHES "Visual Studio")
-   join(netfxpath
-        "C:\\Program Files\\Reference Assemblies\\Microsoft\\Framework\\"
-        ".NETFramework\\v4.0")
--  file(WRITE run-msbuild.bat "
-+  file(WRITE "${CMAKE_BINARY_DIR}/run-msbuild.bat" "
-     ${MSBUILD_SETUP}
-     ${CMAKE_MAKE_PROGRAM} -p:FrameworkPathOverride=\"${netfxpath}\" %*")
- endif ()

dep/vcpkg-overlay-ports/fmt/portfile.cmake

@@ -1,38 +0,0 @@
-vcpkg_from_github(
-    OUT_SOURCE_PATH SOURCE_PATH
-    REPO fmtlib/fmt
-    REF "${VERSION}"
-    SHA512 573b7de1bd224b7b1b60d44808a843db35d4bc4634f72a9edcb52cf68e99ca66c744fd5d5c97b4336ba70b94abdabac5fc253b245d0d5cd8bbe2a096bf941e39
-    HEAD_REF master
-    PATCHES
-        fix-write-batch.patch
-)
-
-vcpkg_cmake_configure(
-    SOURCE_PATH "${SOURCE_PATH}"
-    OPTIONS
-        -DFMT_CMAKE_DIR=share/fmt
-        -DFMT_TEST=OFF
-        -DFMT_DOC=OFF
-        -DFMT_PEDANTIC=ON
-)
-
-vcpkg_cmake_install()
-vcpkg_cmake_config_fixup()
-vcpkg_fixup_pkgconfig()
-vcpkg_copy_pdbs()
-
-if(VCPKG_LIBRARY_LINKAGE STREQUAL dynamic)
-    vcpkg_replace_string("${CURRENT_PACKAGES_DIR}/include/fmt/base.h"
-        "defined(FMT_SHARED)"
-        "1"
-    )
-endif()
-
-file(REMOVE_RECURSE
-    "${CURRENT_PACKAGES_DIR}/debug/include"
-    "${CURRENT_PACKAGES_DIR}/debug/share"
-)
-
-file(INSTALL "${CMAKE_CURRENT_LIST_DIR}/usage" DESTINATION "${CURRENT_PACKAGES_DIR}/share/${PORT}")
-vcpkg_install_copyright(FILE_LIST "${SOURCE_PATH}/LICENSE")

dep/vcpkg-overlay-ports/fmt/usage

@@ -1,8 +0,0 @@
-The package fmt provides CMake targets:
-
-    find_package(fmt CONFIG REQUIRED)
-    target_link_libraries(main PRIVATE fmt::fmt)
-
-    # Or use the header-only version
-    find_package(fmt CONFIG REQUIRED)
-    target_link_libraries(main PRIVATE fmt::fmt-header-only)

dep/vcpkg-overlay-ports/fmt/vcpkg.json

@@ -1,17 +0,0 @@
-{
-  "name": "fmt",
-  "version": "11.1.4",
-  "description": "{fmt} is an open-source formatting library providing a fast and safe alternative to C stdio and C++ iostreams.",
-  "homepage": "https://github.com/fmtlib/fmt",
-  "license": "MIT",
-  "dependencies": [
-    {
-      "name": "vcpkg-cmake",
-      "host": true
-    },
-    {
-      "name": "vcpkg-cmake-config",
-      "host": true
-    }
-  ]
-}

src/buffer/out/Row.cpp

@@ -431,7 +431,7 @@ OutputCellIterator ROW::WriteCells(OutputCellIterator it, const til::CoordType c
     THROW_HR_IF(E_INVALIDARG, limitRight.value_or(0) >= size());
 
     // If we're given a right-side column limit, use it. Otherwise, the write limit is the final column index available in the char row.
-    const auto finalColumnInRow = gsl::narrow_cast<uint16_t>(limitRight.value_or(size() - 1));
+    const auto finalColumnInRow = limitRight.value_or(size() - 1);
 
     auto currentColor = it->TextAttr();
     uint16_t colorUses = 0;

src/cascadia/TerminalApp/AppLogic.cpp

@@ -331,16 +331,8 @@ namespace winrt::TerminalApp::implementation
     void AppLogic::_ApplyLanguageSettingChange() noexcept
     try
     {
-        const auto language = _settings.GlobalSettings().Language();
-
         if (!IsPackaged())
         {
-            if (!language.empty())
-            {
-                // We cannot use the packaged app API, PrimaryLanguageOverride, but we *can* tell the resource loader
-                // to set the Language for all loaded resources to the user's preferred language.
-                winrt::Windows::ApplicationModel::Resources::Core::ResourceContext::SetGlobalQualifierValue(L"Language", language);
-            }
             return;
         }
 
@@ -348,6 +340,8 @@ namespace winrt::TerminalApp::implementation
 
         // NOTE: PrimaryLanguageOverride throws if this instance is unpackaged.
         const auto primaryLanguageOverride = ApplicationLanguages::PrimaryLanguageOverride();
+        const auto language = _settings.GlobalSettings().Language();
+
         if (primaryLanguageOverride != language)
         {
             ApplicationLanguages::PrimaryLanguageOverride(language);

src/cascadia/TerminalApp/CommandPalette.cpp

@@ -530,79 +530,6 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
-    // Method Description:
-    // - This event is called when the user's mouse pointer enters an individual
-    //   item from the list. We'll get the item that was hovered and "preview"
-    //   the command that the user hovered. To do that, we'll dispatch the switch
-    //   to tab command for this tab, but not dismiss the switcher.
-    //
-    // Arguments:
-    // - sender: the UI element that raised the event.
-    // Return Value:
-    // - <none>
-    void CommandPalette::_listItemPointerEntered(const winrt::Windows::Foundation::IInspectable& sender,
-                                                 const winrt::Windows::UI::Xaml::Input::PointerRoutedEventArgs& /*args*/)
-    {
-        // cancel any pending exit timer to prevent an unwanted preview revert
-        if (_pointerExitTimer)
-        {
-            _pointerExitTimer.Stop();
-        }
-
-        const auto listViewItem = sender.try_as<winrt::Windows::UI::Xaml::Controls::ListViewItem>();
-        if (_currentMode == CommandPaletteMode::ActionMode && listViewItem)
-        {
-            const auto enteredItem = listViewItem.Content();
-            if (const auto filteredCommand{ enteredItem.try_as<winrt::TerminalApp::FilteredCommand>() })
-            {
-                if (const auto actionPaletteItem{ filteredCommand.Item().try_as<winrt::TerminalApp::ActionPaletteItem>() })
-                {
-                    // immediately preview the hovered command
-                    PreviewAction.raise(*this, actionPaletteItem.Command());
-                }
-            }
-        }
-    }
-
-    // Method Description:
-    // - This event is called when the user's mouse pointer exits an individual
-    //   item from the list. We then revert to previewing the selected item rather
-    //   than the hovered one, using a short delay (via a DispatcherTimer) to smooth
-    //   transitions when rapidly moving between items.
-    //
-    // Arguments:
-    // - <none>
-    // Return Value:
-    // - <none>
-    void CommandPalette::_listItemPointerExited(const winrt::Windows::Foundation::IInspectable& /*sender*/,
-                                                const winrt::Windows::UI::Xaml::Input::PointerRoutedEventArgs& /*args*/)
-    {
-        // if there is no exit timer, create one
-        if (!_pointerExitTimer)
-        {
-            _pointerExitTimer = winrt::Windows::UI::Xaml::DispatcherTimer();
-            _pointerExitTimer.Interval(std::chrono::milliseconds(10));
-            _pointerExitTimer.Tick([this](auto const&, auto const&) {
-                // when the timer ticks, revert the preview to the selected command
-                const auto selectedCommand = _filteredActionsView().SelectedItem();
-                if (const auto filteredCommand{ selectedCommand.try_as<winrt::TerminalApp::FilteredCommand>() })
-                {
-                    if (_currentMode == CommandPaletteMode::ActionMode && filteredCommand)
-                    {
-                        if (const auto actionPaletteItem{ filteredCommand.Item().try_as<winrt::TerminalApp::ActionPaletteItem>() })
-                        {
-                            PreviewAction.raise(*this, actionPaletteItem.Command());
-                        }
-                    }
-                }
-                _pointerExitTimer.Stop();
-            });
-        }
-
-        // restart the timer
-        _pointerExitTimer.Start();
-    }
-
     void CommandPalette::_listItemSelectionChanged(const Windows::Foundation::IInspectable& /*sender*/, const Windows::UI::Xaml::Controls::SelectionChangedEventArgs& e)
     {
         // We don't care about...
@@ -1287,9 +1214,6 @@ namespace winrt::TerminalApp::implementation
 
         ParentCommandName(L"");
         _currentNestedCommands.Clear();
-
-        // revert any preview
-        _filteredActionsView().SelectedIndex(-1);
         PreviewAction.raise(*this, nullptr);
     }
 
@@ -1382,10 +1306,6 @@ namespace winrt::TerminalApp::implementation
         else
         {
             itemContainer.DataContext(args.Item());
-
-            // attach the pointer event handlers to the container
-            itemContainer.PointerEntered({ this, &CommandPalette::_listItemPointerEntered });
-            itemContainer.PointerExited({ this, &CommandPalette::_listItemPointerExited });
         }
     }
 

src/cascadia/TerminalApp/CommandPalette.h

@@ -78,8 +78,6 @@ namespace winrt::TerminalApp::implementation
 
         Windows::Foundation::Collections::IVector<winrt::TerminalApp::FilteredCommand> _commandsToFilter();
 
-        winrt::Windows::UI::Xaml::DispatcherTimer _pointerExitTimer{ nullptr }; // timer to debounce pointer exit events (used to smooth preview transitions)
-
         bool _lastFilterTextWasEmpty{ true };
 
         void _populateCommands();
@@ -105,10 +103,6 @@ namespace winrt::TerminalApp::implementation
 
         void _listItemClicked(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::Controls::ItemClickEventArgs& e);
 
-        void _listItemPointerEntered(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& args);
-
-        void _listItemPointerExited(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::Input::PointerRoutedEventArgs& args);
-
         void _listItemSelectionChanged(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::Controls::SelectionChangedEventArgs& e);
 
         void _moveBackButtonClicked(const Windows::Foundation::IInspectable& sender, const Windows::UI::Xaml::RoutedEventArgs&);

src/cascadia/TerminalApp/DebugTapConnection.cpp

@@ -60,12 +60,9 @@ namespace winrt::Microsoft::TerminalApp::implementation
 
     DebugTapConnection::DebugTapConnection(ITerminalConnection wrappedConnection)
     {
-        _outputRevoker = wrappedConnection.TerminalOutput(winrt::auto_revoke, { get_weak(), &DebugTapConnection::_OutputHandler });
-        _stateChangedRevoker = wrappedConnection.StateChanged(winrt::auto_revoke, [weak = get_weak()](auto&& /*s*/, auto&& /*e*/) {
-            if (const auto self = weak.get())
-            {
-                self->StateChanged.raise(*self, nullptr);
-            }
+        _outputRevoker = wrappedConnection.TerminalOutput(winrt::auto_revoke, { this, &DebugTapConnection::_OutputHandler });
+        _stateChangedRevoker = wrappedConnection.StateChanged(winrt::auto_revoke, [this](auto&& /*s*/, auto&& /*e*/) {
+            StateChanged.raise(*this, nullptr);
         });
         _wrappedConnection = wrappedConnection;
     }

src/cascadia/TerminalApp/TerminalWindow.cpp

@@ -144,6 +144,7 @@ namespace winrt::TerminalApp::implementation
     {
         // Now that we know we can do XAML, build our page.
         _root = winrt::make_self<TerminalPage>(*_WindowProperties, _manager);
+        _dialog = ContentDialog{};
 
         // Pass in information about the initial state of the window.
         // * If we were supposed to start from serialized "content", do that,
@@ -312,15 +313,6 @@ namespace winrt::TerminalApp::implementation
     {
         return _settings.GlobalSettings().CurrentTheme();
     }
-
-    // WinUI can't show 2 dialogs simultaneously. Yes, really. If you do, you get an exception.
-    // As such, we must dismiss whatever dialog is currently being shown.
-    //
-    // This limit is of course per-thread and not per-window. Yes... really. See:
-    //   https://github.com/microsoft/microsoft-ui-xaml/issues/794
-    // The consequence is that we use a static variable to keep track of the shown dialog.
-    static ContentDialog s_activeDialog{ nullptr };
-
     // Method Description:
     // - Show a ContentDialog with buttons to take further action. Uses the
     //   FrameworkElements provided as the title and content of this dialog, and
@@ -336,32 +328,16 @@ namespace winrt::TerminalApp::implementation
     // - an IAsyncOperation with the dialog result
     winrt::Windows::Foundation::IAsyncOperation<ContentDialogResult> TerminalWindow::ShowDialog(winrt::WUX::Controls::ContentDialog dialog)
     {
-        // As mentioned on s_activeDialog, dismissing the active dialog is necessary.
-        // We repeat it a few times in case the resume_foreground failed to work,
-        // but I found that one iteration will always be enough in practice.
-        for (int i = 0; i < 3; ++i)
-        {
-            if (!s_activeDialog)
-            {
-                break;
-            }
-
-            s_activeDialog.Hide();
-
-            // Wait for the current dialog to be hidden.
-            co_await wil::resume_foreground(_root->Dispatcher(), CoreDispatcherPriority::Low);
-        }
-
-        // If two sources call ShowDialog() simultaneously, it may happen that both enter the above loop,
-        // but it's crucial that only one of them continues below as only 1 dialog can be shown at a time.
-        // Thankfully, everything runs on the UI thread, so only 1 caller will exit the above loop at a time.
-        // So, if s_activeDialog is still set at this point, we must have lost the race.
-        if (s_activeDialog)
+        // DON'T release this lock in a wil::scope_exit. The scope_exit will get
+        // called when we await, which is not what we want.
+        std::unique_lock lock{ _dialogLock, std::try_to_lock };
+        if (!lock)
         {
+            // Another dialog is visible.
             co_return ContentDialogResult::None;
         }
 
-        s_activeDialog = dialog;
+        _dialog = dialog;
 
         // IMPORTANT: This is necessary as documented in the ContentDialog MSDN docs.
         // Since we're hosting the dialog in a Xaml island, we need to connect it to the
@@ -391,26 +367,23 @@ namespace winrt::TerminalApp::implementation
             }
         } };
 
-        auto result = ContentDialogResult::None;
+        themingLambda(dialog, nullptr); // if it's already in the tree
+        auto loadedRevoker{ dialog.Loaded(winrt::auto_revoke, themingLambda) }; // if it's not yet in the tree
 
-        // Extra scope to drop the revoker before resetting the s_activeDialog to null.
-        {
-            themingLambda(dialog, nullptr); // if it's already in the tree
-            auto loadedRevoker{ dialog.Loaded(winrt::auto_revoke, themingLambda) }; // if it's not yet in the tree
-            result = co_await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup);
-        }
+        // Display the dialog.
+        co_return co_await dialog.ShowAsync(Controls::ContentDialogPlacement::Popup);
 
-        s_activeDialog = nullptr;
-        co_return result;
+        // After the dialog is dismissed, the dialog lock (held by `lock`) will
+        // be released so another can be shown
     }
 
     // Method Description:
     // - Dismiss the (only) visible ContentDialog
     void TerminalWindow::DismissDialog()
     {
-        if (s_activeDialog)
+        if (auto localDialog = std::exchange(_dialog, nullptr))
         {
-            s_activeDialog.Hide();
+            localDialog.Hide();
         }
     }
 

src/cascadia/TerminalApp/TerminalWindow.h

@@ -167,6 +167,8 @@ namespace winrt::TerminalApp::implementation
         // ALSO: If you add any UIElements as roots here, make sure they're
         // updated in _ApplyTheme. The root currently is _root.
         winrt::com_ptr<TerminalPage> _root{ nullptr };
+        winrt::Windows::UI::Xaml::Controls::ContentDialog _dialog{ nullptr };
+        std::shared_mutex _dialogLock;
 
         wil::com_ptr<CommandlineArgs> _appArgs{ nullptr };
         bool _hasCommandLineArguments{ false };

src/cascadia/TerminalApp/pch.h

@@ -27,7 +27,6 @@
 
 #include <winrt/Windows.ApplicationModel.h>
 #include <winrt/Windows.ApplicationModel.DataTransfer.h>
-#include <winrt/Windows.ApplicationModel.Resources.Core.h>
 #include <winrt/Windows.Foundation.h>
 #include <winrt/Windows.Foundation.Collections.h>
 #include <winrt/Windows.Foundation.Metadata.h>

src/cascadia/TerminalConnection/CTerminalHandoff.cpp

@@ -14,13 +14,6 @@ static DWORD g_cTerminalHandoffRegistration = 0;
 // Mutex so we only do start/stop/establish one at a time.
 static std::shared_mutex _mtx;
 
-// This is the callback that will be called when a connection is received.
-// Call this once during startup and don't ever change it again (race condition).
-void CTerminalHandoff::s_setCallback(NewHandoffFunction callback) noexcept
-{
-    _pfnHandoff = callback;
-}
-
 // Routine Description:
 // - Starts listening for TerminalHandoff requests by registering
 //   our class and interface with COM.
@@ -28,19 +21,24 @@ void CTerminalHandoff::s_setCallback(NewHandoffFunction callback) noexcept
 // - pfnHandoff - Function to callback when a handoff is received
 // Return Value:
 // - S_OK, E_NOT_VALID_STATE (start called when already started) or relevant COM registration error.
-HRESULT CTerminalHandoff::s_StartListening()
+HRESULT CTerminalHandoff::s_StartListening(NewHandoffFunction pfnHandoff)
 try
 {
     std::unique_lock lock{ _mtx };
 
+    RETURN_HR_IF(E_NOT_VALID_STATE, _pfnHandoff != nullptr);
+
     const auto classFactory = Make<SimpleClassFactory<CTerminalHandoff>>();
-    RETURN_LAST_ERROR_IF_NULL(classFactory);
+
+    RETURN_IF_NULL_ALLOC(classFactory);
 
     ComPtr<IUnknown> unk;
     RETURN_IF_FAILED(classFactory.As(&unk));
 
     RETURN_IF_FAILED(CoRegisterClassObject(__uuidof(CTerminalHandoff), unk.Get(), CLSCTX_LOCAL_SERVER, REGCLS_SINGLEUSE, &g_cTerminalHandoffRegistration));
 
+    _pfnHandoff = pfnHandoff;
+
     return S_OK;
 }
 CATCH_RETURN()
@@ -55,6 +53,15 @@ CATCH_RETURN()
 HRESULT CTerminalHandoff::s_StopListening()
 {
     std::unique_lock lock{ _mtx };
+    return s_StopListeningLocked();
+}
+
+// See s_StopListening()
+HRESULT CTerminalHandoff::s_StopListeningLocked()
+{
+    RETURN_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff);
+
+    _pfnHandoff = nullptr;
 
     if (g_cTerminalHandoffRegistration)
     {
@@ -85,15 +92,22 @@ HRESULT CTerminalHandoff::EstablishPtyHandoff(HANDLE* in, HANDLE* out, HANDLE si
 {
     try
     {
+        std::unique_lock lock{ _mtx };
+
+        // s_StopListeningLocked sets _pfnHandoff to nullptr.
+        // localPfnHandoff is tested for nullness below.
+#pragma warning(suppress : 26429) // Symbol '...' is never tested for nullness, it can be marked as not_null (f.23).
+        auto localPfnHandoff = _pfnHandoff;
+
         // Because we are REGCLS_SINGLEUSE... we need to `CoRevokeClassObject` after we handle this ONE call.
         // COM does not automatically clean that up for us. We must do it.
-        LOG_IF_FAILED(s_StopListening());
+        LOG_IF_FAILED(s_StopListeningLocked());
 
         // Report an error if no one registered a handoff function before calling this.
-        THROW_HR_IF_NULL(E_NOT_VALID_STATE, _pfnHandoff);
+        THROW_HR_IF_NULL(E_NOT_VALID_STATE, localPfnHandoff);
 
         // Call registered handler from when we started listening.
-        THROW_IF_FAILED(_pfnHandoff(in, out, signal, reference, server, client, startupInfo));
+        THROW_IF_FAILED(localPfnHandoff(in, out, signal, reference, server, client, startupInfo));
 
 #pragma warning(suppress : 26477)
         TraceLoggingWrite(

src/cascadia/TerminalConnection/CTerminalHandoff.h

@@ -38,11 +38,11 @@ struct __declspec(uuid(__CLSID_CTerminalHandoff))
 
 #pragma endregion
 
-    static void s_setCallback(NewHandoffFunction callback) noexcept;
-    static HRESULT s_StartListening();
+    static HRESULT s_StartListening(NewHandoffFunction pfnHandoff);
+    static HRESULT s_StopListening();
 
 private:
-    static HRESULT s_StopListening();
+    static HRESULT s_StopListeningLocked();
 };
 
 // Disable warnings from the CoCreatableClass macro as the value it provides for

src/cascadia/TerminalConnection/ConptyConnection.cpp

@@ -780,12 +780,12 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 
     void ConptyConnection::StartInboundListener()
     {
-        static const auto init = []() noexcept {
-            CTerminalHandoff::s_setCallback(&ConptyConnection::NewHandoff);
-            return true;
-        }();
+        THROW_IF_FAILED(CTerminalHandoff::s_StartListening(&ConptyConnection::NewHandoff));
+    }
 
-        CTerminalHandoff::s_StartListening();
+    void ConptyConnection::StopInboundListener()
+    {
+        THROW_IF_FAILED(CTerminalHandoff::s_StopListening());
     }
 
     // Function Description:

src/cascadia/TerminalConnection/ConptyConnection.h

@@ -36,6 +36,7 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
         WORD ShowWindow() const noexcept;
 
         static void StartInboundListener();
+        static void StopInboundListener();
 
         static winrt::event_token NewConnection(const NewConnectionHandler& handler);
         static void NewConnection(const winrt::event_token& token);

src/cascadia/TerminalConnection/ConptyConnection.idl

@@ -23,6 +23,7 @@ namespace Microsoft.Terminal.TerminalConnection
 
         static event NewConnectionHandler NewConnection;
         static void StartInboundListener();
+        static void StopInboundListener();
 
         static Windows.Foundation.Collections.ValueSet CreateSettings(String cmdline,
                                                                       String startingDirectory,

src/cascadia/TerminalControl/ControlCore.cpp

@@ -142,13 +142,23 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // If we wait, a screen reader may try to get the AutomationPeer (aka the UIA Engine), and we won't be able to attach
         // the UIA Engine to the renderer. This prevents us from signaling changes to the cursor or buffer.
         {
+            // First create the render thread.
+            // Then stash a local pointer to the render thread so we can initialize it and enable it
+            // to paint itself *after* we hand off its ownership to the renderer.
+            // We split up construction and initialization of the render thread object this way
+            // because the renderer and render thread have circular references to each other.
+            auto renderThread = std::make_unique<::Microsoft::Console::Render::RenderThread>();
+            auto* const localPointerToThread = renderThread.get();
+
             // Now create the renderer and initialize the render thread.
             const auto& renderSettings = _terminal->GetRenderSettings();
-            _renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(renderSettings, _terminal.get());
+            _renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(renderSettings, _terminal.get(), nullptr, 0, std::move(renderThread));
 
             _renderer->SetBackgroundColorChangedCallback([this]() { _rendererBackgroundColorChanged(); });
             _renderer->SetFrameColorChangedCallback([this]() { _rendererTabColorChanged(); });
             _renderer->SetRendererEnteredErrorStateCallback([this]() { RendererEnteredErrorState.raise(nullptr, nullptr); });
+
+            THROW_IF_FAILED(localPointerToThread->Initialize(_renderer.get()));
         }
 
         UpdateSettings(settings, unfocusedAppearance);
@@ -176,7 +186,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         // thread is a workaround for us to hit GH#12607 less often.
         shared->outputIdle = std::make_unique<til::debounced_func_trailing<>>(
             std::chrono::milliseconds{ 100 },
-            [this, weakThis = get_weak(), dispatcher = _dispatcher]() {
+            [weakTerminal = std::weak_ptr{ _terminal }, weakThis = get_weak(), dispatcher = _dispatcher]() {
                 dispatcher.TryEnqueue(DispatcherQueuePriority::Normal, [weakThis]() {
                     if (const auto self = weakThis.get(); self && !self->_IsClosing())
                     {
@@ -184,23 +194,22 @@ namespace winrt::Microsoft::Terminal::Control::implementation
                     }
                 });
 
-                // We can't use a `weak_ptr` to `_terminal` here, because it takes significant
-                // dependency on the lifetime of `this` (primarily on our `_renderer`).
-                // and a `weak_ptr` would allow it to outlive `this`.
-                // Theoretically `debounced_func_trailing` should call `WaitForThreadpoolTimerCallbacks()`
-                // with cancel=true on destruction, which should ensure that our use of `this` here is safe.
-                const auto lock = _terminal->LockForWriting();
-                _terminal->UpdatePatternsUnderLock();
+                if (const auto t = weakTerminal.lock())
+                {
+                    const auto lock = t->LockForWriting();
+                    t->UpdatePatternsUnderLock();
+                }
             });
 
         // If you rapidly show/hide Windows Terminal, something about GotFocus()/LostFocus() gets broken.
         // We'll then receive easily 10+ such calls from WinUI the next time the application is shown.
         shared->focusChanged = std::make_unique<til::debounced_func_trailing<bool>>(
             std::chrono::milliseconds{ 25 },
-            [this](const bool focused) {
-                // Theoretically `debounced_func_trailing` should call `WaitForThreadpoolTimerCallbacks()`
-                // with cancel=true on destruction, which should ensure that our use of `this` here is safe.
-                _focusChanged(focused);
+            [weakThis = get_weak()](const bool focused) {
+                if (const auto core{ weakThis.get() })
+                {
+                    core->_focusChanged(focused);
+                }
             });
 
         // Scrollbar updates are also expensive (XAML), so we'll throttle them as well.
@@ -215,35 +224,19 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             });
     }
 
-    // Safely disconnects event handlers from the connection and closes it. This is necessary because
-    // WinRT event revokers don't prevent pending calls from proceeding (thread-safe but not race-free).
-    void ControlCore::_closeConnection()
-    {
-        _connectionOutputEventRevoker.revoke();
-        _connectionStateChangedRevoker.revoke();
-
-        // One of the tasks for `ITerminalConnection::Close()` is to block until all pending
-        // callback calls have completed. This solves the race-condition issue mentioned above.
-        if (_connection)
-        {
-            _connection.Close();
-            _connection = nullptr;
-        }
-    }
-
     ControlCore::~ControlCore()
     {
         Close();
 
-        // See notes about the _renderer member in the header file.
-        _renderer->TriggerTeardown();
+        _renderer.reset();
+        _renderEngine.reset();
     }
 
     void ControlCore::Detach()
     {
         // Disable the renderer, so that it doesn't try to start any new frames
         // for our engines while we're not attached to anything.
-        _renderer->TriggerTeardown();
+        _renderer->WaitForPaintCompletionAndDisable(INFINITE);
 
         // Clear out any throttled funcs that we had wired up to run on this UI
         // thread. These will be recreated in _setupDispatcherAndCallbacks, when
@@ -283,7 +276,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         auto oldState = ConnectionState(); // rely on ControlCore's automatic null handling
         // revoke ALL old handlers immediately
 
-        _closeConnection();
+        _connectionOutputEventRevoker.revoke();
+        _connectionStateChangedRevoker.revoke();
 
         _connection = newConnection;
         if (_connection)
@@ -372,11 +366,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             const auto vp = _renderEngine->GetViewportInCharacters(viewInPixels);
             const auto width = vp.Width();
             const auto height = vp.Height();
-
-            if (_connection)
-            {
-                _connection.Resize(height, width);
-            }
+            _connection.Resize(height, width);
 
             if (_owningHwnd != 0)
             {
@@ -430,7 +420,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     {
         if (_initializedTerminal.load(std::memory_order_relaxed))
         {
-            // The lock must be held, because it calls into IRenderData which is shared state.
             const auto lock = _terminal->LockForWriting();
             _renderer->EnablePainting();
         }
@@ -445,10 +434,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
     // - <none>
     void ControlCore::_sendInputToConnection(std::wstring_view wstr)
     {
-        if (_connection)
-        {
-            _connection.WriteInput(winrt_wstring_to_array_view(wstr));
-        }
+        _connection.WriteInput(winrt_wstring_to_array_view(wstr));
     }
 
     // Method Description:
@@ -485,7 +471,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         const wchar_t CtrlD = 0x4;
         const wchar_t Enter = '\r';
 
-        if (_connection && _connection.State() >= winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::Closed)
+        if (_connection.State() >= winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::Closed)
         {
             if (ch == CtrlD)
             {
@@ -1136,10 +1122,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             return;
         }
 
-        if (_connection)
-        {
-            _connection.Resize(vp.Height(), vp.Width());
-        }
+        _connection.Resize(vp.Height(), vp.Width());
 
         // TermControl will call Search() once the OutputIdle even fires after 100ms.
         // Until then we need to hide the now-stale search results from the renderer.
@@ -1811,9 +1794,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
             // Ensure Close() doesn't hang, waiting for MidiAudio to finish playing an hour long song.
             _midiAudio.BeginSkip();
-        }
 
-        _closeConnection();
+            // Stop accepting new output and state changes before we disconnect everything.
+            _connectionOutputEventRevoker.revoke();
+            _connectionStateChangedRevoker.revoke();
+            _connection.Close();
+        }
     }
 
     void ControlCore::PersistToPath(const wchar_t* path) const
@@ -1910,7 +1896,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
         const auto weakThis{ get_weak() };
 
-        // Concurrent read of _dispatcher is safe, because Detach() calls TriggerTeardown()
+        // Concurrent read of _dispatcher is safe, because Detach() calls WaitForPaintCompletionAndDisable()
         // which blocks until this call returns. _dispatcher will only be changed afterwards.
         co_await wil::resume_foreground(_dispatcher);
 
@@ -1961,9 +1947,8 @@ namespace winrt::Microsoft::Terminal::Control::implementation
 
     void ControlCore::ResumeRendering()
     {
-        // The lock must be held, because it calls into IRenderData which is shared state.
         const auto lock = _terminal->LockForWriting();
-        _renderer->EnablePainting();
+        _renderer->ResetErrorStateAndResume();
     }
 
     bool ControlCore::IsVtMouseModeEnabled() const

src/cascadia/TerminalControl/ControlCore.h

@@ -311,8 +311,65 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             std::shared_ptr<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>> updateScrollBar;
         };
 
+        std::atomic<bool> _initializedTerminal{ false };
+        bool _closing{ false };
+
+        TerminalConnection::ITerminalConnection _connection{ nullptr };
+        TerminalConnection::ITerminalConnection::TerminalOutput_revoker _connectionOutputEventRevoker;
+        TerminalConnection::ITerminalConnection::StateChanged_revoker _connectionStateChangedRevoker;
+
+        winrt::com_ptr<ControlSettings> _settings{ nullptr };
+
+        std::shared_ptr<::Microsoft::Terminal::Core::Terminal> _terminal{ nullptr };
+        std::wstring _pendingResponses;
+
+        // NOTE: _renderEngine must be ordered before _renderer.
+        //
+        // As _renderer has a dependency on _renderEngine (through a raw pointer)
+        // we must ensure the _renderer is deallocated first.
+        // (C++ class members are destroyed in reverse order.)
+        std::unique_ptr<::Microsoft::Console::Render::Atlas::AtlasEngine> _renderEngine{ nullptr };
+        std::unique_ptr<::Microsoft::Console::Render::Renderer> _renderer{ nullptr };
+
+        ::Search _searcher;
+        bool _snapSearchResultToSelection;
+
+        winrt::handle _lastSwapChainHandle{ nullptr };
+
+        FontInfoDesired _desiredFont;
+        FontInfo _actualFont;
+        bool _builtinGlyphs = true;
+        bool _colorGlyphs = true;
+        CSSLengthPercentage _cellWidth;
+        CSSLengthPercentage _cellHeight;
+
+        // storage location for the leading surrogate of a utf-16 surrogate pair
+        std::optional<wchar_t> _leadingSurrogate{ std::nullopt };
+
+        std::optional<til::point> _lastHoveredCell{ std::nullopt };
+        // Track the last hyperlink ID we hovered over
+        uint16_t _lastHoveredId{ 0 };
+
+        bool _isReadOnly{ false };
+
+        std::optional<interval_tree::IntervalTree<til::point, size_t>::interval> _lastHoveredInterval{ std::nullopt };
+
+        // These members represent the size of the surface that we should be
+        // rendering to.
+        float _panelWidth{ 0 };
+        float _panelHeight{ 0 };
+        float _compositionScale{ 0 };
+
+        uint64_t _owningHwnd{ 0 };
+
+        winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr };
+        til::shared_mutex<SharedState> _shared;
+
+        til::point _contextMenuBufferPosition{ 0, 0 };
+
+        Windows::Foundation::Collections::IVector<hstring> _cachedQuickFixes{ nullptr };
+
         void _setupDispatcherAndCallbacks();
-        void _closeConnection();
 
         bool _setFontSizeUnderLock(float fontSize);
         void _updateFont();
@@ -339,8 +396,12 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void _terminalWindowSizeChanged(int32_t width, int32_t height);
 
         safe_void_coroutine _terminalCompletionsChanged(std::wstring_view menuJson, unsigned int replaceLength);
+
 #pragma endregion
 
+        MidiAudio _midiAudio;
+        winrt::Windows::System::DispatcherQueueTimer _midiAudioSkipTimer{ nullptr };
+
 #pragma region RendererCallbacks
         void _rendererWarning(const HRESULT hr, wil::zwstring_view parameter);
         safe_void_coroutine _renderEngineSwapChainChanged(const HANDLE handle);
@@ -351,7 +412,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         void _raiseReadOnlyWarning();
         void _updateAntiAliasingMode();
         void _connectionOutputHandler(const hstring& hstr);
-        void _connectionStateChangedHandler(const TerminalConnection::ITerminalConnection&, const Windows::Foundation::IInspectable&);
         void _updateHoveredCell(const std::optional<til::point> terminalPosition);
         void _setOpacity(const float opacity, const bool focused = true);
 
@@ -383,70 +443,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             return _closing;
         }
 
-        // Caches responses generated by our VT parser (= improved batching).
-        std::wstring _pendingResponses;
-
-        // Font stuff.
-        FontInfoDesired _desiredFont;
-        FontInfo _actualFont;
-        bool _builtinGlyphs = true;
-        bool _colorGlyphs = true;
-        CSSLengthPercentage _cellWidth;
-        CSSLengthPercentage _cellHeight;
-
-        // Rendering stuff.
-        winrt::handle _lastSwapChainHandle{ nullptr };
-        uint64_t _owningHwnd{ 0 };
-        float _panelWidth{ 0 };
-        float _panelHeight{ 0 };
-        float _compositionScale{ 0 };
-
-        // Audio stuff.
-        MidiAudio _midiAudio;
-        winrt::Windows::System::DispatcherQueueTimer _midiAudioSkipTimer{ nullptr };
-
-        // Other stuff.
-        winrt::Windows::System::DispatcherQueue _dispatcher{ nullptr };
-        winrt::com_ptr<ControlSettings> _settings{ nullptr };
-        til::point _contextMenuBufferPosition{ 0, 0 };
-        Windows::Foundation::Collections::IVector<hstring> _cachedQuickFixes{ nullptr };
-        ::Search _searcher;
-        std::optional<interval_tree::IntervalTree<til::point, size_t>::interval> _lastHoveredInterval;
-        std::optional<wchar_t> _leadingSurrogate;
-        std::optional<til::point> _lastHoveredCell;
-        uint16_t _lastHoveredId{ 0 };
-        std::atomic<bool> _initializedTerminal{ false };
-        bool _isReadOnly{ false };
-        bool _closing{ false };
-
-        // ----------------------------------------------------------------------------------------
-        // These are ordered last to ensure they're destroyed first.
-        // This ensures that their respective contents stops taking dependency on the above.
-        // I recommend reading the following paragraphs in reverse order.
-        // ----------------------------------------------------------------------------------------
-
-        // ↑ This one is tricky - all of these are raw pointers:
-        //   1. _terminal depends on _renderer (for invalidations)
-        //   2. _renderer depends on _terminal (for IRenderData)
-        //      = circular dependency = architectural flaw (lifetime issues) = TODO
-        //   3. _renderer depends on _renderEngine (AtlasEngine)
-        // To solve the knot, we manually stop the renderer in the destructor,
-        // which breaks 2. We can proceed then proceed to break 1. and then 3.
-        std::unique_ptr<::Microsoft::Console::Render::Atlas::AtlasEngine> _renderEngine{ nullptr }; // 3.
-        std::unique_ptr<::Microsoft::Console::Render::Renderer> _renderer{ nullptr }; // 3.
-        std::shared_ptr<::Microsoft::Terminal::Core::Terminal> _terminal{ nullptr }; // 1.
-
-        // ↑ MOST IMPORTANTLY: `_outputIdle` takes dependency on the raw `this` pointer (necessarily).
-        // Destroying SharedState here will block until all pending `debounced_func_trailing` calls are completed.
-        til::shared_mutex<SharedState> _shared;
-
-        // ↑ Prevent any more unnecessary `_outputIdle` calls.
-        // Technically none of these members are destroyed here. Instead, the destructor will call Close()
-        // which calls _closeConnection() which in turn manually & safely destroys them in the correct order.
-        TerminalConnection::ITerminalConnection::TerminalOutput_revoker _connectionOutputEventRevoker;
-        TerminalConnection::ITerminalConnection::StateChanged_revoker _connectionStateChangedRevoker;
-        TerminalConnection::ITerminalConnection _connection{ nullptr };
-
         friend class ControlUnitTests::ControlCoreTests;
         friend class ControlUnitTests::ControlInteractivityTests;
         bool _inUnitTests{ false };

src/cascadia/TerminalControl/ControlInteractivity.cpp

@@ -73,7 +73,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             //
             // To alleviate, make sure to disable the UIA engine and remove it,
             // and ALSO disable the renderer. Core.Detach will take care of the
-            // TriggerTeardown (which will stop the renderer
+            // WaitForPaintCompletionAndDisable (which will stop the renderer
             // after all current engines are done painting).
             //
             // Simply disabling the UIA engine is not enough, because it's

src/cascadia/TerminalControl/HwndTerminal.cpp

@@ -206,10 +206,14 @@ HRESULT HwndTerminal::Initialize()
     _terminal = std::make_unique<::Microsoft::Terminal::Core::Terminal>();
     const auto lock = _terminal->LockForWriting();
 
+    auto renderThread = std::make_unique<::Microsoft::Console::Render::RenderThread>();
+    auto* const localPointerToThread = renderThread.get();
     auto& renderSettings = _terminal->GetRenderSettings();
     renderSettings.SetColorTableEntry(TextColor::DEFAULT_BACKGROUND, RGB(12, 12, 12));
     renderSettings.SetColorTableEntry(TextColor::DEFAULT_FOREGROUND, RGB(204, 204, 204));
-    _renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(renderSettings, _terminal.get());
+    _renderer = std::make_unique<::Microsoft::Console::Render::Renderer>(renderSettings, _terminal.get(), nullptr, 0, std::move(renderThread));
+    RETURN_HR_IF_NULL(E_POINTER, localPointerToThread);
+    RETURN_IF_FAILED(localPointerToThread->Initialize(_renderer.get()));
 
     auto engine = std::make_unique<::Microsoft::Console::Render::AtlasEngine>();
     RETURN_IF_FAILED(engine->SetHwnd(_hwnd.get()));
@@ -230,7 +234,7 @@ HRESULT HwndTerminal::Initialize()
 
     _terminal->Create({ 80, 25 }, 9001, *_renderer);
     _terminal->SetWriteInputCallback([=](std::wstring_view input) noexcept { _WriteTextToConnection(input); });
-    _renderer->EnablePainting();
+    localPointerToThread->EnablePainting();
 
     _multiClickTime = std::chrono::milliseconds{ GetDoubleClickTime() };
 

src/cascadia/TerminalControl/dll/TerminalControl.vcxproj

@@ -95,7 +95,7 @@
   <ItemDefinitionGroup>
     <Link>
       <AdditionalDependencies>delayimp.lib;Uiautomationcore.lib;onecoreuap.lib;%(AdditionalDependencies)</AdditionalDependencies>
-      <DelayLoadDLLs>uiautomationcore.dll;icu.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
+      <DelayLoadDLLs>uiautomationcore.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
       <!--
       ControlLib contains a DllMain that we need to force the use of.
       If you don't have this, then you'll see an error like

src/cascadia/TerminalSettingsEditor/Launch.xaml

@@ -140,7 +140,8 @@
             </local:SettingContainer>
 
             <!--  Language  -->
-            <local:SettingContainer x:Uid="Globals_Language">
+            <local:SettingContainer x:Uid="Globals_Language"
+                                    Visibility="{x:Bind ViewModel.LanguageSelectorAvailable}">
                 <ComboBox ItemsSource="{x:Bind ViewModel.LanguageList}"
                           SelectedItem="{x:Bind ViewModel.CurrentLanguage, Mode=TwoWay}"
                           Style="{StaticResource ComboBoxSettingStyle}">

src/cascadia/TerminalSettingsEditor/LaunchViewModel.cpp

@@ -82,6 +82,16 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         return language.NativeName();
     }
 
+    // Returns whether the language selector is available/shown.
+    //
+    // winrt::Windows::Globalization::ApplicationLanguages::PrimaryLanguageOverride()
+    // doesn't work for unpackaged applications. The corresponding code in TerminalApp is disabled.
+    // It would be confusing for our users if we presented a dysfunctional language selector.
+    bool LaunchViewModel::LanguageSelectorAvailable()
+    {
+        return IsPackaged();
+    }
+
     // Returns the list of languages the user may override the application language with.
     // The returned list are BCP 47 language tags like {"und", "en-US", "de-DE", "es-ES", ...}.
     // "und" is short for "undefined" and is synonymous for "Use system language" in this code.
@@ -92,6 +102,12 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             return _languageList;
         }
 
+        if (!LanguageSelectorAvailable())
+        {
+            _languageList = {};
+            return _languageList;
+        }
+
         // In order to return the language list this code does the following:
         // [1] Get all possible languages we want to allow the user to choose.
         //     We have to acquire languages from multiple sources, creating duplicates. See below at [1].
@@ -163,24 +179,19 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             return _currentLanguage;
         }
 
-        winrt::hstring currentLanguage;
-        if (IsPackaged())
+        if (!LanguageSelectorAvailable())
         {
-            // NOTE: PrimaryLanguageOverride throws if this instance is unpackaged.
-            currentLanguage = winrt::Windows::Globalization::ApplicationLanguages::PrimaryLanguageOverride();
-        }
-        else
-        {
-            if (_Settings.GlobalSettings().HasLanguage())
-            {
-                currentLanguage = _Settings.GlobalSettings().Language();
-            }
+            _currentLanguage = {};
+            return _currentLanguage;
         }
 
+        // NOTE: PrimaryLanguageOverride throws if this instance is unpackaged.
+        auto currentLanguage = winrt::Windows::Globalization::ApplicationLanguages::PrimaryLanguageOverride();
         if (currentLanguage.empty())
         {
             currentLanguage = systemLanguageTag;
         }
+
         _currentLanguage = winrt::box_value(currentLanguage);
         return _currentLanguage;
     }

src/cascadia/TerminalSettingsEditor/LaunchViewModel.h

@@ -20,6 +20,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         // "Deutsch (Deutschland)". This works independently of the user's locale.
         static winrt::hstring LanguageDisplayConverter(const winrt::hstring& tag);
 
+        bool LanguageSelectorAvailable();
         winrt::Windows::Foundation::Collections::IObservableVector<winrt::hstring> LanguageList();
         winrt::Windows::Foundation::IInspectable CurrentLanguage();
         void CurrentLanguage(const winrt::Windows::Foundation::IInspectable& tag);

src/cascadia/TerminalSettingsEditor/LaunchViewModel.idl

@@ -10,6 +10,7 @@ namespace Microsoft.Terminal.Settings.Editor
     runtimeclass LaunchViewModel : Windows.UI.Xaml.Data.INotifyPropertyChanged
     {
         static String LanguageDisplayConverter(String tag);
+        Boolean LanguageSelectorAvailable { get; };
         Windows.Foundation.Collections.IObservableVector<String> LanguageList { get; };
         IInspectable CurrentLanguage;
 

src/cascadia/WindowsTerminal/WindowEmperor.cpp

@@ -246,8 +246,6 @@ void WindowEmperor::CreateNewWindow(winrt::TerminalApp::WindowRequestedArgs args
 
     auto host = std::make_shared<AppHost>(this, _app.Logic(), std::move(args));
     host->Initialize();
-
-    _windowCount += 1;
     _windows.emplace_back(std::move(host));
 }
 
@@ -356,7 +354,10 @@ void WindowEmperor::HandleCommandlineArgs(int nCmdShow)
         }
 
         // If we created no windows, e.g. because the args are "/?" we can just exit now.
-        _postQuitMessageIfNeeded();
+        if (_windows.empty())
+        {
+            _postQuitMessageIfNeeded();
+        }
     }
 
     // ALWAYS change the _real_ CWD of the Terminal to system32,
@@ -737,14 +738,30 @@ void WindowEmperor::_createMessageWindow(const wchar_t* className)
     StringCchCopy(_notificationIcon.szTip, ARRAYSIZE(_notificationIcon.szTip), appNameLoc.c_str());
 }
 
+// Counterpart to _postQuitMessageIfNeeded:
+// If it returns true, don't close that last window, if any.
+// This ensures we persist the last window.
+bool WindowEmperor::_shouldSkipClosingWindows() const
+{
+    const auto globalSettings = _app.Logic().Settings().GlobalSettings();
+    const size_t windowLimit = globalSettings.ShouldUsePersistedLayout() ? 1 : 0;
+    return _windows.size() <= windowLimit;
+}
+
 // Posts a WM_QUIT as soon as we have no reason to exist anymore.
-// That basically means no windows and no message boxes.
+// That basically means no windows [^1] and no message boxes.
+//
+// [^1] Unless:
+// * We've been asked to persist the last remaining window
+//   in which case we exit with 1 remaining window.
+// * We're allowed to be headless
+//   in which case we never exit.
 void WindowEmperor::_postQuitMessageIfNeeded() const
 {
-    if (
-        _messageBoxCount <= 0 &&
-        _windowCount <= 0 &&
-        !_app.Logic().Settings().GlobalSettings().AllowHeadless())
+    const auto globalSettings = _app.Logic().Settings().GlobalSettings();
+    const size_t windowLimit = globalSettings.ShouldUsePersistedLayout() ? 1 : 0;
+
+    if (_messageBoxCount <= 0 && _windows.size() <= windowLimit && !globalSettings.AllowHeadless())
     {
         PostQuitMessage(0);
     }
@@ -778,20 +795,8 @@ LRESULT WindowEmperor::_messageHandler(HWND window, UINT const message, WPARAM c
         {
         case WM_CLOSE_TERMINAL_WINDOW:
         {
-            const auto globalSettings = _app.Logic().Settings().GlobalSettings();
-            // Keep the last window in the array so that we can persist it on exit.
-            // We check for AllowHeadless(), as that being true prevents us from ever quitting in the first place.
-            // (= If we avoided closing the last window you wouldn't be able to reach a headless state.)
-            const auto shouldKeepWindow =
-                _windows.size() == 1 &&
-                globalSettings.ShouldUsePersistedLayout() &&
-                !globalSettings.AllowHeadless();
-
-            if (!shouldKeepWindow)
+            if (!_shouldSkipClosingWindows())
             {
-                // Did the window counter get out of sync? It shouldn't.
-                assert(_windowCount == gsl::narrow_cast<int32_t>(_windows.size()));
-
                 const auto host = reinterpret_cast<AppHost*>(lParam);
                 auto it = _windows.begin();
                 const auto end = _windows.end();
@@ -807,8 +812,6 @@ LRESULT WindowEmperor::_messageHandler(HWND window, UINT const message, WPARAM c
                 }
             }
 
-            // Counterpart specific to CreateNewWindow().
-            _windowCount -= 1;
             _postQuitMessageIfNeeded();
             return 0;
         }
@@ -925,9 +928,18 @@ void WindowEmperor::_finalizeSessionPersistence() const
 {
     const auto state = ApplicationState::SharedInstance();
 
-    if (_forcePersistence || _app.Logic().Settings().GlobalSettings().ShouldUsePersistedLayout())
+    // If the user started this session with persistence enabled, but then disabled it,
+    // we still need to ensure of course that we clear it out on exit.
+    // In other words, the persisted state must always be nulled out here.
+    //
+    // We check if there's any first, in order to avoid setting the dirty flag on the state.
+    if (state.PersistedWindowLayouts())
     {
         state.PersistedWindowLayouts(nullptr);
+    }
+
+    if (_forcePersistence || _app.Logic().Settings().GlobalSettings().ShouldUsePersistedLayout())
+    {
         for (const auto& w : _windows)
         {
             w->Logic().PersistState();

src/cascadia/WindowsTerminal/WindowEmperor.h

@@ -78,7 +78,6 @@ private:
     bool _forcePersistence = false;
     bool _needsPersistenceCleanup = false;
     std::optional<bool> _currentSystemThemeIsDark;
-    int32_t _windowCount = 0;
     int32_t _messageBoxCount = 0;
 
 #ifdef NDEBUG

src/host/exe/Host.EXE.vcxproj

@@ -87,7 +87,6 @@
     <Link>
       <AllowIsolation>true</AllowIsolation>
       <AdditionalDependencies>winmm.lib;imm32.lib;%(AdditionalDependencies)</AdditionalDependencies>
-      <DelayLoadDLLs>icu.dll;%(DelayLoadDLLs)</DelayLoadDLLs>
     </Link>
   </ItemDefinitionGroup>
   <!-- Careful reordering these. Some default props (contained in these files) are order sensitive. -->

src/host/srvinit.cpp

@@ -842,7 +842,16 @@ PWSTR TranslateConsoleTitle(_In_ PCWSTR pwszConsoleTitle, const BOOL fUnexpand,
     {
         if (!gci.IsInVtIoMode())
         {
-            g.pRender = new Renderer(gci.GetRenderSettings(), &gci.renderData);
+            auto renderThread = std::make_unique<RenderThread>();
+            // stash a local pointer to the thread here -
+            // We're going to give ownership of the thread to the Renderer,
+            //      but the thread also need to be told who its renderer is,
+            //      and we can't do that until the renderer is constructed.
+            auto* const localPointerToThread = renderThread.get();
+
+            g.pRender = new Renderer(gci.GetRenderSettings(), &gci.renderData, nullptr, 0, std::move(renderThread));
+
+            THROW_IF_FAILED(localPointerToThread->Initialize(g.pRender));
 
             // Set up the renderer to be used to calculate the width of a glyph,
             //      should we be unable to figure out its width another way.

src/host/ut_host/ScreenBufferTests.cpp

@@ -44,6 +44,7 @@ class ScreenBufferTests
 
         m_state->InitEvents();
         m_state->PrepareGlobalFont({ 1, 1 });
+        m_state->PrepareGlobalRenderer();
         m_state->PrepareGlobalInputBuffer();
         m_state->PrepareGlobalScreenBuffer();
 
@@ -53,6 +54,7 @@ class ScreenBufferTests
     TEST_CLASS_CLEANUP(ClassCleanup)
     {
         m_state->CleanupGlobalScreenBuffer();
+        m_state->CleanupGlobalRenderer();
         m_state->CleanupGlobalInputBuffer();
 
         delete m_state;
@@ -579,6 +581,8 @@ void ScreenBufferTests::TestResetClearTabStops()
     // Reset the screen buffer to test the defaults.
     m_state->CleanupNewTextBufferInfo();
     m_state->CleanupGlobalScreenBuffer();
+    m_state->CleanupGlobalRenderer();
+    m_state->PrepareGlobalRenderer();
     m_state->PrepareGlobalScreenBuffer();
     m_state->PrepareNewTextBufferInfo();
 

src/host/ut_host/SearchTests.cpp

@@ -23,14 +23,20 @@ class SearchTests
     TEST_CLASS_SETUP(ClassSetup)
     {
         m_state = new CommonState();
+
+        m_state->PrepareGlobalRenderer();
         m_state->PrepareGlobalScreenBuffer();
+
         return true;
     }
 
     TEST_CLASS_CLEANUP(ClassCleanup)
     {
         m_state->CleanupGlobalScreenBuffer();
+        m_state->CleanupGlobalRenderer();
+
         delete m_state;
+
         return true;
     }
 

src/inc/test/CommonState.hpp

@@ -66,6 +66,20 @@ public:
         m_pFontInfo = { L"Consolas", 0, 0, coordFontSize, 0 };
     }
 
+    void PrepareGlobalRenderer()
+    {
+        Globals& g = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals();
+        CONSOLE_INFORMATION& gci = g.getConsoleInformation();
+        g.pRender = new Microsoft::Console::Render::Renderer(gci.GetRenderSettings(), &gci.renderData, nullptr, 0, nullptr);
+    }
+
+    void CleanupGlobalRenderer()
+    {
+        Globals& g = Microsoft::Console::Interactivity::ServiceLocator::LocateGlobals();
+        delete g.pRender;
+        g.pRender = nullptr;
+    }
+
     void PrepareGlobalScreenBuffer(const til::CoordType viewWidth = s_csWindowWidth,
                                    const til::CoordType viewHeight = s_csWindowHeight,
                                    const til::CoordType bufferWidth = s_csBufferWidth,

src/interactivity/onecore/ConIoSrvComm.cpp

@@ -393,58 +393,7 @@ VOID ConIoSrvComm::HandleFocusEvent(const CIS_EVENT* const Event)
             {
                 // Wait for the currently running paint operation, if any,
                 // and prevent further attempts to render.
-                //
-                // When rendering takes place via DirectX, and a console application
-                // currently owns the screen, and a new console application is launched (or
-                // the user switches to another console application), the new application
-                // cannot take over the screen until the active one relinquishes it. This
-                // blocking mechanism goes as follows:
-                //
-                // 1. The console input thread of the new console application connects to
-                // ConIoSrv;
-                // 2. While servicing the new connection request, ConIoSrv sends an event to
-                // the active application letting it know that it has lost focus;
-                // 3.1 ConIoSrv waits for a reply from the client application;
-                // 3.2 Meanwhile, the active application receives the focus event and calls
-                // this method, waiting for the current paint operation to
-                // finish.
-                //
-                // This means that the new application is waiting on the connection request
-                // reply from ConIoSrv, ConIoSrv is waiting on the active application to
-                // acknowledge the lost focus event to reply to the new application, and the
-                // console input thread in the active application is waiting on the renderer
-                // thread to finish its current paint operation.
-                //
-                // Question: what should happen if the wait on the paint operation times
-                // out?
-                //
-                // There are three options:
-                //
-                // 1. On timeout, the active console application could reply with an error
-                // message and terminate itself, effectively relinquishing control of the
-                // display;
-                //
-                // 2. ConIoSrv itself could time out on waiting for a reply, and forcibly
-                // terminate the active console application;
-                //
-                // 3. Let the wait time out and let the user deal with it. Because the wait
-                // occurs on a single iteration of the renderer thread, it seemed to me that
-                // the likelihood of failure is extremely small, especially since the client
-                // console application that the active conhost instance is servicing has no
-                // say over what happens in the renderer thread, only by proxy. Thus, the
-                // chance of failure (timeout) is minimal and since the OneCoreUAP console
-                // is not a massively used piece of software, it didn’t seem that it would
-                // be a good use of time to build the requisite infrastructure to deal with
-                // a timeout here, at least not for now. In case of a timeout DirectX will
-                // catch the mistake of a new application attempting to acquire the display
-                // while another one still owns it and will flag it as a DWM bug. Right now,
-                // the active application will wait one second for the paint operation to
-                // finish.
-                //
-                // TODO: MSFT: 11833883 - Determine action when wait on paint operation via
-                //       DirectX on OneCoreUAP times out while switching console
-                //       applications.
-                Renderer->TriggerTeardown();
+                Renderer->WaitForPaintCompletionAndDisable(1000);
 
                 // Relinquish control of the graphics device (only one
                 // DirectX application may control the device at any one

src/renderer/base/renderer.cpp

@@ -25,12 +25,35 @@ static constexpr auto renderBackoffBaseTimeMilliseconds{ 150 };
 // - Creates a new renderer controller for a console.
 // Arguments:
 // - pData - The interface to console data structures required for rendering
+// - pEngine - The output engine for targeting each rendering frame
 // Return Value:
 // - An instance of a Renderer.
-Renderer::Renderer(const RenderSettings& renderSettings, IRenderData* pData) :
+Renderer::Renderer(const RenderSettings& renderSettings,
+                   IRenderData* pData,
+                   _In_reads_(cEngines) IRenderEngine** const rgpEngines,
+                   const size_t cEngines,
+                   std::unique_ptr<RenderThread> thread) :
     _renderSettings(renderSettings),
-    _pData(pData)
+    _pData(pData),
+    _pThread{ std::move(thread) }
 {
+    for (size_t i = 0; i < cEngines; i++)
+    {
+        AddRenderEngine(rgpEngines[i]);
+    }
+}
+
+// Routine Description:
+// - Destroys an instance of a renderer
+// Arguments:
+// - <none>
+// Return Value:
+// - <none>
+Renderer::~Renderer()
+{
+    // RenderThread blocks until it has shut down.
+    _destructing = true;
+    _pThread.reset();
 }
 
 IRenderData* Renderer::GetRenderData() const noexcept
@@ -49,6 +72,11 @@ IRenderData* Renderer::GetRenderData() const noexcept
     auto tries = maxRetriesForRenderEngine;
     while (tries > 0)
     {
+        if (_destructing)
+        {
+            return S_FALSE;
+        }
+
         // BODGY: Optimally we would want to retry per engine, but that causes different
         // problems (intermittent inconsistent states between text renderer and UIA output,
         // not being able to lock the cursor location, etc.).
@@ -63,7 +91,7 @@ IRenderData* Renderer::GetRenderData() const noexcept
         if (--tries == 0)
         {
             // Stop trying.
-            _thread.DisablePainting();
+            _pThread->DisablePainting();
             if (_pfnRendererEnteredErrorState)
             {
                 _pfnRendererEnteredErrorState();
@@ -179,8 +207,12 @@ CATCH_RETURN()
 
 void Renderer::NotifyPaintFrame() noexcept
 {
-    // The thread will provide throttling for us.
-    _thread.NotifyPaint();
+    // If we're running in the unittests, we might not have a render thread.
+    if (_pThread)
+    {
+        // The thread will provide throttling for us.
+        _pThread->NotifyPaint();
+    }
 }
 
 // Routine Description:
@@ -283,7 +315,7 @@ void Renderer::TriggerRedrawAll(const bool backgroundChanged, const bool frameCh
 void Renderer::TriggerTeardown() noexcept
 {
     // We need to shut down the paint thread on teardown.
-    _thread.TriggerTeardown();
+    _pThread->WaitForPaintCompletionAndDisable(INFINITE);
 }
 
 // Routine Description:
@@ -605,7 +637,25 @@ void Renderer::EnablePainting()
     // When the renderer is constructed, the initial viewport won't be available yet,
     // but once EnablePainting is called it should be safe to retrieve.
     _viewport = _pData->GetViewport();
-    _thread.EnablePainting();
+
+    // When running the unit tests, we may be using a render without a render thread.
+    if (_pThread)
+    {
+        _pThread->EnablePainting();
+    }
+}
+
+// Routine Description:
+// - Waits for the current paint operation to complete, if any, up to the specified timeout.
+// - Resets an event in the render thread that precludes it from advancing, thus disabling rendering.
+// - If no paint operation is currently underway, returns immediately.
+// Arguments:
+// - dwTimeoutMs - Milliseconds to wait for the current paint operation to complete, if any (can be INFINITE).
+// Return Value:
+// - <none>
+void Renderer::WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs)
+{
+    _pThread->WaitForPaintCompletionAndDisable(dwTimeoutMs);
 }
 
 // Routine Description:
@@ -1387,6 +1437,14 @@ void Renderer::SetRendererEnteredErrorStateCallback(std::function<void()> pfn)
     _pfnRendererEnteredErrorState = std::move(pfn);
 }
 
+// Method Description:
+// - Attempts to restart the renderer.
+void Renderer::ResetErrorStateAndResume()
+{
+    // because we're not stateful (we could be in the future), all we want to do is reenable painting.
+    EnablePainting();
+}
+
 void Renderer::UpdateHyperlinkHoveredId(uint16_t id) noexcept
 {
     _hyperlinkHoveredId = id;

src/renderer/base/renderer.hpp

@@ -28,7 +28,13 @@ namespace Microsoft::Console::Render
     class Renderer
     {
     public:
-        Renderer(const RenderSettings& renderSettings, IRenderData* pData);
+        Renderer(const RenderSettings& renderSettings,
+                 IRenderData* pData,
+                 _In_reads_(cEngines) IRenderEngine** const pEngine,
+                 const size_t cEngines,
+                 std::unique_ptr<RenderThread> thread);
+
+        ~Renderer();
 
         IRenderData* GetRenderData() const noexcept;
 
@@ -65,6 +71,7 @@ namespace Microsoft::Console::Render
         bool IsGlyphWideByFont(const std::wstring_view glyph);
 
         void EnablePainting();
+        void WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs);
         void WaitUntilCanRender();
 
         void AddRenderEngine(_In_ IRenderEngine* const pEngine);
@@ -73,6 +80,7 @@ namespace Microsoft::Console::Render
         void SetBackgroundColorChangedCallback(std::function<void()> pfn);
         void SetFrameColorChangedCallback(std::function<void()> pfn);
         void SetRendererEnteredErrorStateCallback(std::function<void()> pfn);
+        void ResetErrorStateAndResume();
 
         void UpdateHyperlinkHoveredId(uint16_t id) noexcept;
         void UpdateLastHoveredInterval(const std::optional<interval_tree::IntervalTree<til::point, size_t>::interval>& newInterval);
@@ -113,25 +121,23 @@ namespace Microsoft::Console::Render
         const RenderSettings& _renderSettings;
         std::array<IRenderEngine*, 2> _engines{};
         IRenderData* _pData = nullptr; // Non-ownership pointer
+        std::unique_ptr<RenderThread> _pThread;
         static constexpr size_t _firstSoftFontChar = 0xEF20;
         size_t _lastSoftFontChar = 0;
         uint16_t _hyperlinkHoveredId = 0;
         std::optional<interval_tree::IntervalTree<til::point, size_t>::interval> _hoveredInterval;
         Microsoft::Console::Types::Viewport _viewport;
-        CursorOptions _currentCursorOptions{};
+        CursorOptions _currentCursorOptions;
         std::optional<CompositionCache> _compositionCache;
         std::vector<Cluster> _clusterBuffer;
         std::function<void()> _pfnBackgroundColorChanged;
         std::function<void()> _pfnFrameColorChanged;
         std::function<void()> _pfnRendererEnteredErrorState;
+        bool _destructing = false;
         bool _forceUpdateViewport = false;
 
         til::point_span _lastSelectionPaintSpan{};
         size_t _lastSelectionPaintSize{};
         std::vector<til::rect> _lastSelectionRectsByViewport{};
-
-        // Ordered last, so that it gets destroyed first.
-        // This ensures that the render thread stops accessing us.
-        RenderThread _thread{ this };
     };
 }

src/renderer/base/thread.cpp

@@ -10,40 +10,212 @@
 
 using namespace Microsoft::Console::Render;
 
-RenderThread::RenderThread(Renderer* renderer) :
-    renderer(renderer)
+RenderThread::RenderThread() :
+    _pRenderer(nullptr),
+    _hThread(nullptr),
+    _hEvent(nullptr),
+    _hPaintCompletedEvent(nullptr),
+    _fKeepRunning(true),
+    _hPaintEnabledEvent(nullptr),
+    _fNextFrameRequested(false),
+    _fWaiting(false)
 {
 }
 
 RenderThread::~RenderThread()
 {
-    TriggerTeardown();
+    if (_hThread)
+    {
+        _fKeepRunning = false; // stop loop after final run
+        EnablePainting(); // if we want to get the last frame out, we need to make sure it's enabled
+        SignalObjectAndWait(_hEvent, _hThread, INFINITE, FALSE); // signal final paint and wait for thread to finish.
+
+        CloseHandle(_hThread);
+        _hThread = nullptr;
+    }
+
+    if (_hEvent)
+    {
+        CloseHandle(_hEvent);
+        _hEvent = nullptr;
+    }
+
+    if (_hPaintEnabledEvent)
+    {
+        CloseHandle(_hPaintEnabledEvent);
+        _hPaintEnabledEvent = nullptr;
+    }
+
+    if (_hPaintCompletedEvent)
+    {
+        CloseHandle(_hPaintCompletedEvent);
+        _hPaintCompletedEvent = nullptr;
+    }
+}
+
+// Method Description:
+// - Create all of the Events we'll need, and the actual thread we'll be doing
+//      work on.
+// Arguments:
+// - pRendererParent: the Renderer that owns this thread, and which we should
+//      trigger frames for.
+// Return Value:
+// - S_OK if we succeeded, else an HRESULT corresponding to a failure to create
+//      an Event or Thread.
+[[nodiscard]] HRESULT RenderThread::Initialize(Renderer* const pRendererParent) noexcept
+{
+    _pRenderer = pRendererParent;
+
+    auto hr = S_OK;
+    // Create event before thread as thread will start immediately.
+    if (SUCCEEDED(hr))
+    {
+        auto hEvent = CreateEventW(nullptr, // non-inheritable security attributes
+                                   FALSE, // auto reset event
+                                   FALSE, // initially unsignaled
+                                   nullptr // no name
+        );
+
+        if (hEvent == nullptr)
+        {
+            hr = HRESULT_FROM_WIN32(GetLastError());
+        }
+        else
+        {
+            _hEvent = hEvent;
+        }
+    }
+
+    if (SUCCEEDED(hr))
+    {
+        auto hPaintEnabledEvent = CreateEventW(nullptr,
+                                               TRUE, // manual reset event
+                                               FALSE, // initially signaled
+                                               nullptr);
+
+        if (hPaintEnabledEvent == nullptr)
+        {
+            hr = HRESULT_FROM_WIN32(GetLastError());
+        }
+        else
+        {
+            _hPaintEnabledEvent = hPaintEnabledEvent;
+        }
+    }
+
+    if (SUCCEEDED(hr))
+    {
+        auto hPaintCompletedEvent = CreateEventW(nullptr,
+                                                 TRUE, // manual reset event
+                                                 TRUE, // initially signaled
+                                                 nullptr);
+
+        if (hPaintCompletedEvent == nullptr)
+        {
+            hr = HRESULT_FROM_WIN32(GetLastError());
+        }
+        else
+        {
+            _hPaintCompletedEvent = hPaintCompletedEvent;
+        }
+    }
+
+    if (SUCCEEDED(hr))
+    {
+        auto hThread = CreateThread(nullptr, // non-inheritable security attributes
+                                    0, // use default stack size
+                                    s_ThreadProc,
+                                    this,
+                                    0, // create immediately
+                                    nullptr // we don't need the thread ID
+        );
+
+        if (hThread == nullptr)
+        {
+            hr = HRESULT_FROM_WIN32(GetLastError());
+        }
+        else
+        {
+            _hThread = hThread;
+
+            // SetThreadDescription only works on 1607 and higher. If we cannot find it,
+            // then it's no big deal. Just skip setting the description.
+            auto func = GetProcAddressByFunctionDeclaration(GetModuleHandleW(L"kernel32.dll"), SetThreadDescription);
+            if (func)
+            {
+                LOG_IF_FAILED(func(hThread, L"Rendering Output Thread"));
+            }
+        }
+    }
+
+    return hr;
 }
 
 DWORD WINAPI RenderThread::s_ThreadProc(_In_ LPVOID lpParameter)
 {
     const auto pContext = static_cast<RenderThread*>(lpParameter);
-    return pContext->_ThreadProc();
+
+    if (pContext != nullptr)
+    {
+        return pContext->_ThreadProc();
+    }
+    else
+    {
+        return (DWORD)E_INVALIDARG;
+    }
 }
 
 DWORD WINAPI RenderThread::_ThreadProc()
 {
-    while (true)
+    while (_fKeepRunning)
     {
-        _enable.wait();
-
         // Between waiting on _hEvent and calling PaintFrame() there should be a minimal delay,
         // so that a key press progresses to a drawing operation as quickly as possible.
         // As such, we wait for the renderer to complete _before_ waiting on _hEvent.
-        renderer->WaitUntilCanRender();
+        _pRenderer->WaitUntilCanRender();
 
-        _redraw.wait();
-        if (!_keepRunning.load(std::memory_order_relaxed))
+        WaitForSingleObject(_hPaintEnabledEvent, INFINITE);
+
+        if (!_fNextFrameRequested.exchange(false, std::memory_order_acq_rel))
         {
-            break;
+            // <--
+            // If `NotifyPaint` is called at this point, then it will not
+            // set the event because `_fWaiting` is not `true` yet so we have
+            // to check again below.
+
+            _fWaiting.store(true, std::memory_order_release);
+
+            // check again now (see comment above)
+            if (!_fNextFrameRequested.exchange(false, std::memory_order_acq_rel))
+            {
+                // Wait until a next frame is requested.
+                WaitForSingleObject(_hEvent, INFINITE);
+            }
+
+            // <--
+            // If `NotifyPaint` is called at this point, then it _will_ set
+            // the event because `_fWaiting` is `true`, but we're not waiting
+            // anymore!
+            // This can probably happen quite often: imagine a scenario where
+            // we are waiting, and the terminal calls `NotifyPaint` twice
+            // very quickly.
+            // In that case, both calls might end up calling `SetEvent`. The
+            // first one will resume this thread and the second one will
+            // `SetEvent` the event. So the next time we wait, the event will
+            // already be set and we won't actually wait.
+            // Because it can happen often, and because rendering is an
+            // expensive operation, we should reset the event to not render
+            // again if nothing changed.
+
+            _fWaiting.store(false, std::memory_order_release);
+
+            // see comment above
+            ResetEvent(_hEvent);
         }
 
-        LOG_IF_FAILED(renderer->PaintFrame());
+        ResetEvent(_hPaintCompletedEvent);
+        LOG_IF_FAILED(_pRenderer->PaintFrame());
+        SetEvent(_hPaintCompletedEvent);
     }
 
     return S_OK;
@@ -51,54 +223,79 @@ DWORD WINAPI RenderThread::_ThreadProc()
 
 void RenderThread::NotifyPaint() noexcept
 {
-    _redraw.SetEvent();
+    if (_fWaiting.load(std::memory_order_acquire))
+    {
+        SetEvent(_hEvent);
+    }
+    else
+    {
+        _fNextFrameRequested.store(true, std::memory_order_release);
+    }
 }
 
-// Spawns a new rendering thread if none exists yet.
 void RenderThread::EnablePainting() noexcept
 {
-    const auto guard = _threadMutex.lock_exclusive();
-
-    _enable.SetEvent();
-
-    if (!_thread)
-    {
-        _keepRunning.store(true, std::memory_order_relaxed);
-
-        _thread.reset(CreateThread(nullptr, 0, s_ThreadProc, this, 0, nullptr));
-        THROW_LAST_ERROR_IF(!_thread);
-
-        // SetThreadDescription only works on 1607 and higher. If we cannot find it,
-        // then it's no big deal. Just skip setting the description.
-        const auto func = GetProcAddressByFunctionDeclaration(GetModuleHandleW(L"kernel32.dll"), SetThreadDescription);
-        if (func)
-        {
-            LOG_IF_FAILED(func(_thread.get(), L"Rendering Output Thread"));
-        }
-    }
+    SetEvent(_hPaintEnabledEvent);
 }
 
 void RenderThread::DisablePainting() noexcept
 {
-    _enable.ResetEvent();
+    ResetEvent(_hPaintEnabledEvent);
 }
 
-// Stops the rendering thread, and waits for it to finish.
-void RenderThread::TriggerTeardown() noexcept
+void RenderThread::WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs) noexcept
 {
-    const auto guard = _threadMutex.lock_exclusive();
+    // When rendering takes place via DirectX, and a console application
+    // currently owns the screen, and a new console application is launched (or
+    // the user switches to another console application), the new application
+    // cannot take over the screen until the active one relinquishes it. This
+    // blocking mechanism goes as follows:
+    //
+    // 1. The console input thread of the new console application connects to
+    // ConIoSrv;
+    // 2. While servicing the new connection request, ConIoSrv sends an event to
+    // the active application letting it know that it has lost focus;
+    // 3.1 ConIoSrv waits for a reply from the client application;
+    // 3.2 Meanwhile, the active application receives the focus event and calls
+    // this method, waiting for the current paint operation to
+    // finish.
+    //
+    // This means that the new application is waiting on the connection request
+    // reply from ConIoSrv, ConIoSrv is waiting on the active application to
+    // acknowledge the lost focus event to reply to the new application, and the
+    // console input thread in the active application is waiting on the renderer
+    // thread to finish its current paint operation.
+    //
+    // Question: what should happen if the wait on the paint operation times
+    // out?
+    //
+    // There are three options:
+    //
+    // 1. On timeout, the active console application could reply with an error
+    // message and terminate itself, effectively relinquishing control of the
+    // display;
+    //
+    // 2. ConIoSrv itself could time out on waiting for a reply, and forcibly
+    // terminate the active console application;
+    //
+    // 3. Let the wait time out and let the user deal with it. Because the wait
+    // occurs on a single iteration of the renderer thread, it seemed to me that
+    // the likelihood of failure is extremely small, especially since the client
+    // console application that the active conhost instance is servicing has no
+    // say over what happens in the renderer thread, only by proxy. Thus, the
+    // chance of failure (timeout) is minimal and since the OneCoreUAP console
+    // is not a massively used piece of software, it didn’t seem that it would
+    // be a good use of time to build the requisite infrastructure to deal with
+    // a timeout here, at least not for now. In case of a timeout DirectX will
+    // catch the mistake of a new application attempting to acquire the display
+    // while another one still owns it and will flag it as a DWM bug. Right now,
+    // the active application will wait one second for the paint operation to
+    // finish.
+    //
+    // TODO: MSFT: 11833883 - Determine action when wait on paint operation via
+    //       DirectX on OneCoreUAP times out while switching console
+    //       applications.
 
-    if (_thread)
-    {
-        // The render thread first waits for the event and then checks _keepRunning. By doing it
-        // in reverse order here, we ensure that it's impossible for the render thread to miss this.
-        _keepRunning.store(false, std::memory_order_relaxed);
-        _redraw.SetEvent();
-        _enable.SetEvent();
-
-        WaitForSingleObject(_thread.get(), INFINITE);
-        _thread.reset();
-    }
-
-    DisablePainting();
+    ResetEvent(_hPaintEnabledEvent);
+    WaitForSingleObject(_hPaintCompletedEvent, dwTimeoutMs);
 }

src/renderer/base/thread.hpp

@@ -21,23 +21,30 @@ namespace Microsoft::Console::Render
     class RenderThread
     {
     public:
-        RenderThread(Renderer* renderer);
+        RenderThread();
         ~RenderThread();
 
+        [[nodiscard]] HRESULT Initialize(Renderer* const pRendererParent) noexcept;
+
         void NotifyPaint() noexcept;
         void EnablePainting() noexcept;
         void DisablePainting() noexcept;
-        void TriggerTeardown() noexcept;
+        void WaitForPaintCompletionAndDisable(const DWORD dwTimeoutMs) noexcept;
 
     private:
         static DWORD WINAPI s_ThreadProc(_In_ LPVOID lpParameter);
         DWORD WINAPI _ThreadProc();
 
-        Renderer* renderer;
-        wil::slim_event_manual_reset _enable;
-        wil::slim_event_auto_reset _redraw;
-        wil::srwlock _threadMutex;
-        wil::unique_handle _thread;
-        std::atomic<bool> _keepRunning{ false };
+        HANDLE _hThread;
+        HANDLE _hEvent;
+
+        HANDLE _hPaintEnabledEvent;
+        HANDLE _hPaintCompletedEvent;
+
+        Renderer* _pRenderer; // Non-ownership pointer
+
+        bool _fKeepRunning;
+        std::atomic<bool> _fNextFrameRequested;
+        std::atomic<bool> _fWaiting;
     };
 }

src/renderer/gdi/paint.cpp

@@ -574,8 +574,7 @@ try
         }
 
         const auto cpt = gsl::narrow_cast<DWORD>(points.size());
-        RETURN_HR_IF(E_FAIL, !PolyBezier(_hdcMemoryContext, points.data(), cpt));
-        return S_OK;
+        return PolyBezier(_hdcMemoryContext, points.data(), cpt);
     };
 
     if (lines.test(GridLines::Left))

src/renderer/inc/DummyRenderer.hpp

@@ -18,7 +18,7 @@ class DummyRenderer final : public Microsoft::Console::Render::Renderer
 {
 public:
     DummyRenderer(Microsoft::Console::Render::IRenderData* pData = nullptr) :
-        Microsoft::Console::Render::Renderer(_renderSettings, pData) {}
+        Microsoft::Console::Render::Renderer(_renderSettings, pData, nullptr, 0, nullptr) {}
 
     Microsoft::Console::Render::RenderSettings _renderSettings;
 };

src/terminal/adapter/SixelParser.cpp

@@ -610,7 +610,7 @@ void SixelParser::_updateTextColors()
     // the text output as well.
     if (_conformanceLevel <= 3 && _maxColors > 2 && _colorTableChanged) [[unlikely]]
     {
-        for (IndexType tableIndex = 0; _maxColors <= 16 && tableIndex < _maxColors; tableIndex++)
+        for (IndexType tableIndex = 0; tableIndex < _maxColors; tableIndex++)
         {
             _dispatcher.SetColorTableEntry(tableIndex, _colorFromIndex(tableIndex));
         }

src/terminal/adapter/charsets.hpp

@@ -19,7 +19,7 @@ namespace Microsoft::Console::VirtualTerminal
     public:
         constexpr CharSet(const std::initializer_list<std::pair<wchar_t, wchar_t>> replacements)
         {
-            for (auto i = L'\0'; i < gsl::narrow_cast<wchar_t>(_translationTable.size()); i++)
+            for (auto i = L'\0'; i < _translationTable.size(); i++)
                 _translationTable.at(i) = BaseChar + i;
             for (auto replacement : replacements)
                 _translationTable.at(replacement.first - BaseChar) = replacement.second;

src/terminal/parser/InputStateMachineEngine.cpp

@@ -471,22 +471,15 @@ bool InputStateMachineEngine::ActionCsiDispatch(const VTID id, const VTParameter
         // We catch it here and store the information for later retrieval.
         if (_deviceAttributes.load(std::memory_order_relaxed) == 0)
         {
-            til::enumset<DeviceAttribute, uint64_t> attributes{ DeviceAttribute::__some__ };
+            til::enumset<DeviceAttribute, uint64_t> attributes;
 
             // The first parameter denotes the conformance level.
-            const auto len = parameters.size();
-            if (len >= 2 && parameters.at(0).value() >= 61)
+            if (parameters.at(0).value() >= 61)
             {
-                // NOTE: VTParameters::for_each will replace empty spans with a single default value.
-                // This means we could not distinguish between no parameters and a single default parameter.
-                for (size_t i = 1; i < len; i++)
-                {
-                    const auto value = parameters.at(i).value();
-                    if (value > 0 && value < 64)
-                    {
-                        attributes.set(static_cast<DeviceAttribute>(value));
-                    }
-                }
+                parameters.subspan(1).for_each([&](auto p) {
+                    attributes.set(static_cast<DeviceAttribute>(p));
+                    return true;
+                });
             }
 
             _deviceAttributes.fetch_or(attributes.bits(), std::memory_order_relaxed);

src/terminal/parser/InputStateMachineEngine.hpp

@@ -51,10 +51,6 @@ namespace Microsoft::Console::VirtualTerminal
 
     enum class DeviceAttribute : uint64_t
     {
-        // Special value to indicate that InputStateMachineEngine::_deviceAttributes has been set.
-        // 0 in this case means 1<<0 == 1, which in turn means that _deviceAttributes is non-zero.
-        __some__ = 0,
-
         Columns132 = 1,
         PrinterPort = 2,
         Sixel = 4,

vcpkg.json

@@ -17,7 +17,7 @@
   "overrides": [
     {
       "name": "fmt",
-      "version": "11.1.4"
+      "version": "11.0.2"
     },
     {
       "name": "ms-gsl",
@@ -36,8 +36,5 @@
       "version": "0.30.3"
     }
   ],
-  "builtin-baseline": "fe1cde61e971d53c9687cf9a46308f8f55da19fa",
-  "vcpkg-configuration": {
-    "overlay-ports": [ "./dep/vcpkg-overlay-ports" ]
-  }
+  "builtin-baseline": "fe1cde61e971d53c9687cf9a46308f8f55da19fa"
 }