Fix a ControlCore race condition on connection close (#13882)

As noted by the `winrt::event` documentation:
> [...] But for asynchronous events, even after revoking [...], an in-flight
> event might reach your object after it has started destructing.

This is because while adding/removing/calling event handlers might be
thread-safe, there's no guarantee that they run mutually exclusive.

This commit fixes the issue by reverting 6f0f245. Since we never checked
the result of closing a terminal connection anyways, this commit simply drops
the wait on the connection being teared down to ensure #1996 doesn't regress.

Closes #13880

## Validation Steps Performed
* Open tab, close tab, open tab, close tab, open tab, close tab
  * ConPTY ✅
  * Azure ✅
* Closing a tab with a huge amount of panes ✅
* Opening a bunch of tabs and then closing the window ✅
* Closing a tab while it's busy with VT ✅
* `wtd -w 0 nt cmd /c exit` ✅
* `wtd -w -1 cmd /c exit`
  * No WerFault spawns ✅

(cherry picked from commit 666c446)
Service-Card-Id: 86178273
Service-Version: 1.15

src/cascadia/TerminalConnection/AzureConnection.cpp

@@ -264,13 +264,14 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
             if (_state == AzureState::TermConnected)
             {
                 // Close the websocket connection
-                auto closedTask = _cloudShellSocket.close();
-                closedTask.wait();
+                _cloudShellSocket.close();
             }
 
             if (_hOutputThread)
             {
-                // Tear down our output thread
+                // Waiting for the output thread to exit ensures that all pending _TerminalOutputHandlers()
+                // calls have returned and won't notify our caller (ControlCore) anymore. This ensures that
+                // we don't call a destroyed event handler asynchronously from a background thread (GH#13880).
                 WaitForSingleObject(_hOutputThread.get(), INFINITE);
                 _hOutputThread.reset();
             }

src/cascadia/TerminalConnection/ConptyConnection.cpp

@@ -535,7 +535,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
         if (_transitionToState(ConnectionState::Closing))
         {
             // EXIT POINT
-            _clientExitWait.reset(); // immediately stop waiting for the client to exit.
+
+            // _clientExitWait holds a CreateThreadpoolWait() which holds a weak reference to "this".
+            // This manual reset() ensures we wait for it to be teared down via WaitForThreadpoolWaitCallbacks().
+            _clientExitWait.reset();
 
             _hPC.reset(); // tear down the pseudoconsole (this is like clicking X on a console window)
 
@@ -544,19 +547,13 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
 
             if (_hOutputThread)
             {
-                // Tear down our output thread -- now that the output pipe was closed on the
-                // far side, we can run down our local reader.
+                // Waiting for the output thread to exit ensures that all pending _TerminalOutputHandlers()
+                // calls have returned and won't notify our caller (ControlCore) anymore. This ensures that
+                // we don't call a destroyed event handler asynchronously from a background thread (GH#13880).
                 LOG_LAST_ERROR_IF(WAIT_FAILED == WaitForSingleObject(_hOutputThread.get(), INFINITE));
                 _hOutputThread.reset();
             }
 
-            if (_piClient.hProcess)
-            {
-                // Wait for the client to terminate (which it should do successfully)
-                LOG_LAST_ERROR_IF(WAIT_FAILED == WaitForSingleObject(_piClient.hProcess, INFINITE));
-                _piClient.reset();
-            }
-
             _transitionToState(ConnectionState::Closed);
         }
     }

src/cascadia/TerminalControl/ControlCore.cpp

@@ -1472,32 +1472,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         _FoundMatchHandlers(*this, *foundResults);
     }
 
-    // Method Description:
-    // - Asynchronously close our connection. The Connection will likely wait
-    //   until the attached process terminates before Close returns. If that's
-    //   the case, we don't want to block the UI thread waiting on that process
-    //   handle.
-    // Arguments:
-    // - <none>
-    // Return Value:
-    // - <none>
-    winrt::fire_and_forget ControlCore::_asyncCloseConnection()
-    {
-        if (auto localConnection{ std::exchange(_connection, nullptr) })
-        {
-            // Close the connection on the background thread.
-            co_await winrt::resume_background(); // ** DO NOT INTERACT WITH THE CONTROL CORE AFTER THIS LINE **
-
-            // Here, the ControlCore very well might be gone.
-            // _asyncCloseConnection is called on the dtor, so it's entirely
-            // possible that the background thread is resuming after we've been
-            // cleaned up.
-
-            localConnection.Close();
-            // connection is destroyed.
-        }
-    }
-
     void ControlCore::Close()
     {
         if (!_IsClosing())
@@ -1507,13 +1481,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation
             // Stop accepting new output and state changes before we disconnect everything.
             _connection.TerminalOutput(_connectionOutputEventToken);
             _connectionStateChangedRevoker.revoke();
-
-            // GH#1996 - Close the connection asynchronously on a background
-            // thread.
-            // Since TermControl::Close is only ever triggered by the UI, we
-            // don't really care to wait for the connection to be completely
-            // closed. We can just do it whenever.
-            _asyncCloseConnection();
+            _connection.Close();
         }
     }
 

src/cascadia/TerminalControl/ControlCore.h

@@ -269,8 +269,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation
         std::unique_ptr<til::throttled_func_trailing<>> _updatePatternLocations;
         std::shared_ptr<ThrottledFuncTrailing<Control::ScrollPositionChangedArgs>> _updateScrollBar;
 
-        winrt::fire_and_forget _asyncCloseConnection();
-
         bool _setFontSizeUnderLock(int fontSize);
         void _updateFont(const bool initialUpdate = false);
         void _refreshSizeUnderLock();