diff --git a/src/cascadia/TerminalConnection/AzureConnection.cpp b/src/cascadia/TerminalConnection/AzureConnection.cpp index 21ec8639f6ce..f04e08bd74b8 100644 --- a/src/cascadia/TerminalConnection/AzureConnection.cpp +++ b/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(); } diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index 2f0b55ca9c55..6a2ae7c8a43f 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -556,19 +556,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); } } diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 3fdeb2ae4423..91057b830410 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -1498,32 +1498,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: - // - - // Return Value: - // - - 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()) @@ -1533,13 +1507,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(); } } diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 63983f9be01d..c82f93406745 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -271,8 +271,6 @@ namespace winrt::Microsoft::Terminal::Control::implementation std::unique_ptr> _updatePatternLocations; std::shared_ptr> _updateScrollBar; - winrt::fire_and_forget _asyncCloseConnection(); - bool _setFontSizeUnderLock(int fontSize); void _updateFont(const bool initialUpdate = false); void _refreshSizeUnderLock();