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>
This commit is contained in:
Marc Mutz 2024-07-09 16:48:01 +02:00 committed by Qt Cherry-pick Bot
parent d2730bc303
commit 4e26c36171
2 changed files with 10 additions and 0 deletions

View File

@ -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;

View File

@ -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