Skip to content

Commit e5e9042

Browse files
committed
fix: Fix freeze when closing terminal on newer Windows 11
Windows 11 24H2 changed the underlying implementation and the "ClosePseudoConsole" code that we copied from the Terminal application didn't work as it did before. The new Terminal releases also removes the WaitForSingleObject call. See microsoft/terminal#17704 Signed-off-by: xiaoming <[email protected]>
1 parent f0aa7db commit e5e9042

File tree

3 files changed

+52
-64
lines changed

3 files changed

+52
-64
lines changed

lib/ptyqt/conptyprocess.cpp

Lines changed: 37 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -574,34 +574,11 @@ void _ClosePseudoConsoleMembers(_In_ PseudoConsole* pPty)
574574
{
575575
if (pPty != nullptr)
576576
{
577-
// See MSFT:19918626
578-
// First break the signal pipe - this will trigger conhost to tear itself down
579577
if (_HandleIsValid(pPty->hSignal))
580578
{
581579
CloseHandle(pPty->hSignal);
582580
pPty->hSignal = nullptr;
583581
}
584-
// Then, wait on the conhost process before killing it.
585-
// We do this to make sure the conhost finishes flushing any output it
586-
// has yet to send before we hard kill it.
587-
if (_HandleIsValid(pPty->hConPtyProcess))
588-
{
589-
// If the conhost is already dead, then that's fine. Presumably
590-
// it's finished flushing it's output already.
591-
DWORD dwExit = 0;
592-
// If GetExitCodeProcess failed, it's likely conhost is already dead
593-
// If so, skip waiting regardless of whatever error
594-
// GetExitCodeProcess returned.
595-
// We'll just go straight to killing conhost.
596-
if (GetExitCodeProcess(pPty->hConPtyProcess, &dwExit) && dwExit == STILL_ACTIVE)
597-
{
598-
WaitForSingleObject(pPty->hConPtyProcess, INFINITE);
599-
}
600-
601-
TerminateProcess(pPty->hConPtyProcess, 0);
602-
CloseHandle(pPty->hConPtyProcess);
603-
pPty->hConPtyProcess = nullptr;
604-
}
605582
// Then take care of the reference handle.
606583
// TODO GH#1810: Closing the reference handle late leaves conhost thinking
607584
// that we have an outstanding connected client.
@@ -610,6 +587,11 @@ void _ClosePseudoConsoleMembers(_In_ PseudoConsole* pPty)
610587
CloseHandle(pPty->hPtyReference);
611588
pPty->hPtyReference = nullptr;
612589
}
590+
if (_HandleIsValid(pPty->hConPtyProcess))
591+
{
592+
CloseHandle(pPty->hConPtyProcess);
593+
pPty->hConPtyProcess = nullptr;
594+
}
613595
}
614596
}
615597

@@ -995,8 +977,11 @@ bool ConPtyProcess::startProcess(const QString &executable,
995977
GetExitCodeProcess(hEvent, &exitCode);
996978
m_exitCode = exitCode;
997979
// Do not respawn if the object is about to be destructed
998-
if (!m_aboutToDestruct)
999-
emit notifier()->aboutToClose();
980+
if (!m_aboutToDestruct) {
981+
ConptyClosePseudoConsole(m_ptyHandler);
982+
m_ptyHandler = INVALID_HANDLE_VALUE;
983+
emit notifier() -> aboutToClose();
984+
}
1000985
m_shellCloseWaitNotifier->setEnabled(false);
1001986
}, Qt::QueuedConnection);
1002987

@@ -1054,49 +1039,45 @@ bool ConPtyProcess::resize(qint16 cols, qint16 rows)
10541039

10551040
bool ConPtyProcess::kill()
10561041
{
1057-
bool exitCode = false;
1058-
10591042
if (m_ptyHandler != INVALID_HANDLE_VALUE) {
10601043
m_aboutToDestruct = true;
10611044

10621045
// Close ConPTY - this will terminate client process if running
10631046
WindowsContext::instance().closePseudoConsole(m_ptyHandler);
1047+
}
10641048

1065-
// Clean-up the pipes
1066-
if (INVALID_HANDLE_VALUE != m_hPipeOut)
1067-
CloseHandle(m_hPipeOut);
1068-
if (INVALID_HANDLE_VALUE != m_hPipeIn)
1069-
CloseHandle(m_hPipeIn);
1070-
1071-
if (m_readThread) {
1072-
m_readThread->requestInterruption();
1073-
if (!m_readThread->wait(1000))
1074-
m_readThread->terminate();
1075-
m_readThread->deleteLater();
1076-
m_readThread = nullptr;
1077-
}
1078-
1079-
delete m_shellCloseWaitNotifier;
1080-
m_shellCloseWaitNotifier = nullptr;
1049+
// Clean-up the pipes
1050+
if (INVALID_HANDLE_VALUE != m_hPipeOut)
1051+
CloseHandle(m_hPipeOut);
1052+
if (INVALID_HANDLE_VALUE != m_hPipeIn)
1053+
CloseHandle(m_hPipeIn);
1054+
1055+
if (m_readThread) {
1056+
m_readThread->requestInterruption();
1057+
if (!m_readThread->wait(1000))
1058+
m_readThread->terminate();
1059+
m_readThread->deleteLater();
1060+
m_readThread = nullptr;
1061+
}
10811062

1082-
m_pid = 0;
1083-
m_ptyHandler = INVALID_HANDLE_VALUE;
1084-
m_hPipeIn = INVALID_HANDLE_VALUE;
1085-
m_hPipeOut = INVALID_HANDLE_VALUE;
1063+
delete m_shellCloseWaitNotifier;
1064+
m_shellCloseWaitNotifier = nullptr;
10861065

1087-
CloseHandle(m_shellProcessInformation.hThread);
1088-
CloseHandle(m_shellProcessInformation.hProcess);
1066+
m_pid = 0;
1067+
m_ptyHandler = INVALID_HANDLE_VALUE;
1068+
m_hPipeIn = INVALID_HANDLE_VALUE;
1069+
m_hPipeOut = INVALID_HANDLE_VALUE;
10891070

1090-
// Cleanup attribute list
1091-
if (m_shellStartupInfo.lpAttributeList) {
1092-
DeleteProcThreadAttributeList(m_shellStartupInfo.lpAttributeList);
1093-
HeapFree(GetProcessHeap(), 0, m_shellStartupInfo.lpAttributeList);
1094-
}
1071+
CloseHandle(m_shellProcessInformation.hThread);
1072+
CloseHandle(m_shellProcessInformation.hProcess);
10951073

1096-
exitCode = true;
1074+
// Cleanup attribute list
1075+
if (m_shellStartupInfo.lpAttributeList) {
1076+
DeleteProcThreadAttributeList(m_shellStartupInfo.lpAttributeList);
1077+
HeapFree(GetProcessHeap(), 0, m_shellStartupInfo.lpAttributeList);
10971078
}
10981079

1099-
return exitCode;
1080+
return true;
11001081
}
11011082

11021083
IPtyProcess::PtyType ConPtyProcess::type()
@@ -1208,10 +1189,6 @@ ConPtyProcess::pidTree_t ConPtyProcess::processInfoTree()
12081189

12091190
bool ConPtyProcess::isAvailable()
12101191
{
1211-
#ifdef TOO_OLD_WINSDK
1212-
return false; //very importnant! ConPty can be built, but it doesn't work if built with old sdk and Win10 < 1903
1213-
#endif
1214-
12151192
qint32 buildNumber = QSysInfo::kernelVersion().split(".").last().toInt();
12161193
if (buildNumber < CONPTY_MINIMAL_WINDOWS_VERSION)
12171194
return false;

lib/ptyqt/conptyprocess.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919
ProcThreadAttributeValue(22, FALSE, TRUE, FALSE)
2020

2121
typedef VOID* HPCON;
22-
23-
#define TOO_OLD_WINSDK
2422
#endif
2523

2624
class QWinEventNotifier;

src/sessionswindow/sessionswindow.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -848,8 +848,21 @@ int SessionsWindow::startLocalShellSession(const QString &command,QString profil
848848
}
849849
connect(localShell->notifier(), &QIODevice::readyRead, this, [=](){
850850
QByteArray data = localShell->readAll();
851-
if(doRecvData(data)) {
852-
term->recvData(data.data(), data.size());
851+
if (!data.isEmpty()) {
852+
if(doRecvData(data)) {
853+
term->recvData(data.data(), data.size());
854+
}
855+
}
856+
});
857+
connect(localShell->notifier(), &QIODevice::aboutToClose, this, [this] {
858+
if (localShell) {
859+
QByteArray restOfOutput = localShell->readAll();
860+
if (!restOfOutput.isEmpty()) {
861+
if(doRecvData(restOfOutput)) {
862+
term->recvData(restOfOutput.data(), restOfOutput.size());
863+
}
864+
localShell->notifier()->disconnect();
865+
}
853866
}
854867
});
855868
connect(this,&SessionsWindow::modemProxySendData,this,[=](QByteArray data){

0 commit comments

Comments
 (0)