Repository: microsoft/terminal (language: cpp)

Issue:
Terminal crashes when closing any other pane than the last
### Windows Terminal version

1.21.1272.0

### Windows build number

10.0.22631.3593

### Other Software

_No response_

### Steps to reproduce

open terminal. split horizontally twice. change to middle pane. CTRL-D -- crash.

i have mouse tracking turned on if relevant

![term-crash](https://github.com/microsoft/terminal/assets/46322585/fbb48d78-ecf0-4ced-8d41-5995c179b7ce)


### Expected Behavior

obviously, no crash :)

### Actual Behavior

it crashes and takes "a long time" to start


Discussion / hints:
Hi I'm an AI powered bot that finds similar issues based off the issue title.

Please view the issues below to see if they solve your problem, and if the issue describes your problem please consider closing this one and thumbs upping the other issue to help us prioritize it. Thank you!


### Closed similar issues:
- [terminal crashes when closing a tab after opening a new tab (#1666)](https://github.com/microsoft/terminal/issues/1666),  similarity score: 0.77
- [Ctrl+D Enter in powershell after closing wsl bash tab with Ctrl+D crashes the terminal. (#2981)](https://github.com/microsoft/terminal/issues/2981),  similarity score: 0.76

> Note: You can give me feedback by thumbs upping or thumbs downing this comment.
It seems to be anything that's not the 'last' pane. I just tried it again and successfully closed the bottom (third) pane

![term-crash2](https://github.com/microsoft/terminal/assets/46322585/26387ec3-212e-4f1a-aee6-7a4f79c68a32)


Happens when using powershell too (not ubuntu). I actually LIKE the change to use the active (not default) profile but I need to be able to close the panes :D

Fix patch:
diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index fd195d8d42a..899e67e14ec 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -467,7 +467,7 @@ std::shared_ptr<Pane> Pane::NextPane(const std::shared_ptr<Pane> targetPane)
     std::shared_ptr<Pane> nextPane = nullptr;
     auto foundTarget = false;
 
-    auto foundNext = WalkTree([&](auto pane) {
+    auto foundNext = WalkTree([&](const auto& pane) {
         // If we are a parent pane we don't want to move to one of our children
         if (foundTarget && targetPane->_HasChild(pane))
         {
@@ -985,6 +985,17 @@ void Pane::_ContentLostFocusHandler(const winrt::Windows::Foundation::IInspectab
 // - <none>
 void Pane::Close()
 {
+    // Pane has two events, CloseRequested and Closed. CloseRequested is raised by the content asking to be closed,
+    // but also by the window who owns the tab when it's closing. The event is then caught by the TerminalTab which
+    // calls Close() which then raises the Closed event. Now, if this is the last pane in the window, this will result
+    // in the window raising CloseRequested again which leads to infinite recursion, so we need to guard against that.
+    // Ideally we would have just a single event in the future.
+    if (_closed)
+    {
+        return;
+    }
+
+    _closed = true;
     // Fire our Closed event to tell our parent that we should be removed.
     Closed.raise(nullptr, nullptr);
 }
@@ -1341,7 +1352,7 @@ std::shared_ptr<Pane> Pane::DetachPane(std::shared_ptr<Pane> pane)
         detached->_ApplySplitDefinitions();
 
         // Trigger the detached event on each child
-        detached->WalkTree([](auto pane) {
+        detached->WalkTree([](const auto& pane) {
             pane->Detached.raise(pane);
         });
 
@@ -1542,7 +1553,7 @@ void Pane::_CloseChild(const bool closeFirst)
         {
             // update our path to our first remaining leaf
             _parentChildPath = _firstChild;
-            _firstChild->WalkTree([](auto p) {
+            _firstChild->WalkTree([](const auto& p) {
                 if (p->_IsLeaf())
                 {
                     return true;
@@ -2398,7 +2409,7 @@ void Pane::Id(uint32_t id) noexcept
 bool Pane::FocusPane(const uint32_t id)
 {
     // Always clear the parent child path if we are focusing a leaf
-    return WalkTree([=](auto p) {
+    return WalkTree([=](const auto& p) {
         p->_parentChildPath.reset();
         if (p->_id == id)
         {
@@ -2421,7 +2432,7 @@ bool Pane::FocusPane(const uint32_t id)
 // - true if focus was set
 bool Pane::FocusPane(const std::shared_ptr<Pane> pane)
 {
-    return WalkTree([&](auto p) {
+    return WalkTree([&](const auto& p) {
         if (p == pane)
         {
             p->_Focus();
diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h
index f25a7b0834a..86757ad36f9 100644
--- a/src/cascadia/TerminalApp/Pane.h
+++ b/src/cascadia/TerminalApp/Pane.h
@@ -248,7 +248,7 @@ class Pane : public std::enable_shared_from_this<Pane>
 
     std::optional<uint32_t> _id;
     std::weak_ptr<Pane> _parentChildPath{};
-
+    bool _closed{ false };
     bool _lastActive{ false };
     winrt::event_token _firstClosedToken{ 0 };
     winrt::event_token _secondClosedToken{ 0 };
diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
index 3053d746df2..8ef5a36584e 100644
--- a/src/cascadia/TerminalApp/TabManagement.cpp
+++ b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -739,7 +739,7 @@ namespace winrt::TerminalApp::implementation
             }
 
             // Clean read-only mode to prevent additional prompt if closing the pane triggers closing of a hosting tab
-            pane->WalkTree([](auto p) {
+            pane->WalkTree([](const auto& p) {
                 if (const auto control{ p->GetTerminalControl() })
                 {
                     if (control.ReadOnly())
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index ca86bb80a37..54a3e4efe09 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -3810,7 +3810,7 @@ namespace winrt::TerminalApp::implementation
             // recipe for disaster. We won't ever open up a tab in this window.
             newTerminalArgs.Elevate(false);
             const auto newPane = _MakePane(newTerminalArgs, nullptr, connection);
-            newPane->WalkTree([](auto pane) {
+            newPane->WalkTree([](const auto& pane) {
                 pane->FinalizeConfigurationGivenDefault();
             });
             _CreateNewTabFromPane(newPane);
diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp
index d3e52d4037d..6e1be17e38b 100644
--- a/src/cascadia/TerminalApp/TerminalTab.cpp
+++ b/src/cascadia/TerminalApp/TerminalTab.cpp
@@ -41,7 +41,7 @@ namespace winrt::TerminalApp::implementation
 
         auto firstId = _nextPaneId;
 
-        _rootPane->WalkTree([&](std::shared_ptr<Pane> pane) {
+        _rootPane->WalkTree([&](const auto& pane) {
             // update the IDs on each pane
             if (pane->_IsLeaf())
             {
@@ -203,7 +203,7 @@ namespace winrt::TerminalApp::implementation
     {
         ASSERT_UI_THREAD();
 
-        _rootPane->WalkTree([&](std::shared_ptr<Pane> pane) {
+        _rootPane->WalkTree([&](const auto& pane) {
             // Attach event handlers to each new pane
             _AttachEventHandlersToPane(pane);
             if (auto content = pane->GetContent())
@@ -275,7 +275,7 @@ namespace winrt::TerminalApp::implementation
         _UpdateHeaderControlMaxWidth();
 
         // Update the settings on all our panes.
-        _rootPane->WalkTree([&](auto pane) {
+        _rootPane->WalkTree([&](const auto& pane) {
             pane->UpdateSettings(settings);
             return false;
         });
@@ -534,7 +534,7 @@ namespace winrt::TerminalApp::implementation
 
         // Add the new event handlers to the new pane(s)
         // and update their ids.
-        pane->WalkTree([&](auto p) {
+        pane->WalkTree([&](const auto& p) {
             _AttachEventHandlersToPane(p);
             if (p->_IsLeaf())
             {
@@ -624,7 +624,7 @@ namespace winrt::TerminalApp::implementation
         // manually.
         _rootPane->Closed(_rootClosedToken);
         auto p = _rootPane;
-        p->WalkTree([](auto pane) {
+        p->WalkTree([](const auto& pane) {
             pane->Detached.raise(pane);
         });
 
@@ -650,7 +650,7 @@ namespace winrt::TerminalApp::implementation
 
         // Add the new event handlers to the new pane(s)
         // and update their ids.
-        pane->WalkTree([&](auto p) {
+        pane->WalkTree([&](const auto& p) {
             _AttachEventHandlersToPane(p);
             if (p->_IsLeaf())
             {
@@ -949,26 +949,20 @@ namespace winrt::TerminalApp::implementation
 
         events.CloseRequested = content.CloseRequested(
             winrt::auto_revoke,
-            [dispatcher, weakThis](auto sender, auto&&) -> winrt::fire_and_forget {
-                // Don't forget! this ^^^^^^^^ sender can't be a reference, this is a async callback.
-
-                // The lambda lives in the `std::function`-style container owned by `control`. That is, when the
-                // `control` gets destroyed the lambda struct also gets destroyed. In other words, we need to
-                // copy `weakThis` onto the stack, because that's the only thing that gets captured in coroutines.
-                // See: https://devblogs.microsoft.com/oldnewthing/20211103-00/?p=105870
-                const auto weakThisCopy = weakThis;
-                co_await wil::resume_foreground(dispatcher);
-                // Check if Tab's lifetime has expired
-                if (auto tab{ weakThisCopy.get() })
+            [this](auto&& sender, auto&&) {
+                if (const auto content{ sender.try_as<TerminalApp::IPaneContent>() })
                 {
-                    if (const auto content{ sender.try_as<TerminalApp::IPaneContent>() })
+                    // Calling Close() while walking the tree is not safe, because Close() mutates the tree.
+                    const auto pane = _rootPane->_FindPane([&](const auto& p) -> std::shared_ptr<Pane> {
+                        if (p->GetContent() == content)
+                        {
+                            return p;
+                        }
+                        return {};
+                    });
+                    if (pane)
                     {
-                        tab->_rootPane->WalkTree([content](std::shared_ptr<Pane> pane) {
-                            if (pane->GetContent() == content)
-                            {
-                                pane->Close();
-                            }
-                        });
+                        pane->Close();
                     }
                 }
             });


Test patch: