From 4e26c36171fb91c7ded57f6a6bf5f84a89603093 Mon Sep 17 00:00:00 2001
From: Marc Mutz <marc.mutz@qt.io>
Date: Tue, 9 Jul 2024 16:48:01 +0200
Subject: [PATCH] QThread: fix race condition between parallel terminate()
 calls

QThread::terminate() is documented to be thread-safe, but had a race
condition: If multiple threads call terminate() on the same thread,
the following could happen:

  T1                     T2

  t0->terminate();
    lock();
    read ID;
    pthread_cancel(ID);
    unlock()
                         t0->terminate();
                           lock();
                           read ID;
    (OS thread finishes)
  t3->start();
    (creates a new OS
     thread with same ID)
                           pthread_cancel(ID); // cancels new t3!
                           unlock();

To fix, record that the thread was already terminated using a new
boolean flag.

An alternative would have been to fetchAndSet() the threadId to nullptr
and only let the thread that actually nulled it call pthread_cancel(),
but that would be harder to restore to the previous state in case
pthread_cancel() fails, and a null threadId might cause other problems
down the line, esp. if cancellation is currently disabled. The
explicit state is much simpler to reason about.

Fixes: QTBUG-127055
Pick-to: 6.7 6.5 6.2 5.15
Change-Id: Iec180bdfaaf913a3a1560210c781966dc99c0d42
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit d8bd4c2306f2acfefc75f8163b58f2037596dc65)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/corelib/thread/qthread_p.h      | 3 +++
 src/corelib/thread/qthread_unix.cpp | 7 +++++++
 2 files changed, 10 insertions(+)

diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h
index 4918fd7b852..e429362514d 100644
--- a/src/corelib/thread/qthread_p.h
+++ b/src/corelib/thread/qthread_p.h
@@ -188,6 +188,9 @@ public:
     bool finished;
     bool isInFinish; //when in QThreadPrivate::finish
     std::atomic<bool> interruptionRequested = false;
+#ifdef Q_OS_UNIX
+    bool terminated = false; // when (the first) terminate has been called
+#endif
 
     bool exited;
     int returnCode;
diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp
index de18bcb0969..a56a764eed5 100644
--- a/src/corelib/thread/qthread_unix.cpp
+++ b/src/corelib/thread/qthread_unix.cpp
@@ -647,6 +647,7 @@ void QThread::start(Priority priority)
     d->returnCode = 0;
     d->exited = false;
     d->interruptionRequested.store(false, std::memory_order_relaxed);
+    d->terminated = false;
 
     pthread_attr_t attr;
     pthread_attr_init(&attr);
@@ -758,8 +759,14 @@ void QThread::terminate()
     if (!d->data->threadId.loadRelaxed())
         return;
 
+    if (d->terminated) // don't try again, avoids killing the wrong thread on threadId reuse (ABA)
+        return;
+
+    d->terminated = true;
+
     int code = pthread_cancel(from_HANDLE<pthread_t>(d->data->threadId.loadRelaxed()));
     if (code) {
+        d->terminated = false; // allow to try again
         qErrnoWarning(code, "QThread::start: Thread termination error");
     }
 #endif