Windows: Don't terminate threads in Qt

Terminating a thread that we don't have full control over might leave
mutexes or critical sections locked, ending up with an application that
can't allocate memory or open new windows.

Also, if terminating of the thread would fail (which the code tried to
handle), then deleting the QThread anyway would have triggered the
assertion that we don't delete a running thread in ~QThread.

So simplify this code: wait simply returns true if the thread isn't
running anymore, no need for the double-check. Leave the thread running
and leaking if it is stuck somewhere in Windows APIs while executing the
native dialog.

Fixes: QTBUG-103984
Change-Id: I34aa42cbde7c769a58c14bf524781cf3abd13b70
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
This commit is contained in:
Volker Hilsheimer 2022-06-02 17:28:51 +02:00
parent f67d89ebde
commit c450285c41

View File

@ -203,18 +203,16 @@ QWindowsDialogHelperBase<BaseClass>::~QWindowsDialogHelperBase()
template <class BaseClass> template <class BaseClass>
void QWindowsDialogHelperBase<BaseClass>::cleanupThread() void QWindowsDialogHelperBase<BaseClass>::cleanupThread()
{ {
if (m_thread) { // Thread may be running if the dialog failed to close. if (m_thread) {
if (m_thread->isRunning()) // Thread may be running if the dialog failed to close. Give it a bit
m_thread->wait(500); // to exit, but let it be a memory leak if that fails. We must not
if (m_thread->isRunning()) { // terminate the thread, it might be stuck in Comdlg32 or an IModalWindow
m_thread->terminate(); // implementation, and we might end up dead-locking the application if the thread
m_thread->wait(300); // holds a mutex or critical section.
if (m_thread->isRunning()) if (m_thread->wait(500))
qCCritical(lcQpaDialogs) <<__FUNCTION__ << "Failed to terminate thread."; delete m_thread;
else else
qCWarning(lcQpaDialogs) << __FUNCTION__ << "Thread terminated."; qCCritical(lcQpaDialogs) <<__FUNCTION__ << "Thread failed to finish.";
}
delete m_thread;
m_thread = nullptr; m_thread = nullptr;
} }
} }