From 272c0215c2058bc267abf0a247da878f652eaffa Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Thu, 11 Jan 2024 11:09:26 +0100
Subject: [PATCH] QThread::terminate(): don't depend on stack unwinding

Posix doesn't seem to specify whether the stack of cancelled threads
is unwound, and there's nothing preventing a QThread from
terminate()ing itself, so be extra careful to drop the mutex before
calling pthread_cancel.

We can't drop the mutex in general, as that would open a window for
the following race condition:

   T1                       T2
   t3->terminate()
     lock();
     read ID;
     terminated = true;
     unlock();
   ----------- t3 exits naturally -----------
                            t3->wait();
                            t4->start(); // gets ex-t3's ID
     pthread_cancel(ID) // oops, cancels new t4

But we can drop it when this == currentThread(), because said window
does not exist: While this_thread is executing terminate(), it cannot
at the same time exit naturally.

As drive-by, scope a variable tighter.

Pick-to: 6.8 6.7 6.5
Change-Id: I77a628e62d88e383d5aa91cfd97440186c997fc4
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
---
 src/corelib/thread/qthread_unix.cpp | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp
index 4158e8aecbc..a4dd7aa8af8 100644
--- a/src/corelib/thread/qthread_unix.cpp
+++ b/src/corelib/thread/qthread_unix.cpp
@@ -751,7 +751,8 @@ void QThread::terminate()
     Q_D(QThread);
     QMutexLocker locker(&d->mutex);
 
-    if (!d->data->threadId.loadRelaxed())
+    const auto id = d->data->threadId.loadRelaxed();
+    if (!id)
         return;
 
     if (d->terminated) // don't try again, avoids killing the wrong thread on threadId reuse (ABA)
@@ -759,8 +760,18 @@ void QThread::terminate()
 
     d->terminated = true;
 
-    int code = pthread_cancel(from_HANDLE<pthread_t>(d->data->threadId.loadRelaxed()));
-    if (code) {
+    const bool selfCancelling = d->data == currentThreadData;
+    if (selfCancelling) {
+        // Posix doesn't seem to specify whether the stack of cancelled threads
+        // is unwound, and there's nothing preventing a QThread from
+        // terminate()ing itself, so drop the mutex before calling
+        // pthread_cancel():
+        locker.unlock();
+    }
+
+    if (int code = pthread_cancel(from_HANDLE<pthread_t>(id))) {
+        if (selfCancelling)
+            locker.relock();
         d->terminated = false; // allow to try again
         qErrnoWarning(code, "QThread::start: Thread termination error");
     }