Skip to content

QThread: fix race condition between parallel terminate() calls

Andreas Sturmlechner requested to merge asturmlechner/qtbase:asturmlechner/QTBUG-127055-Qthread-ABA into kde/5.15

Not completely sure about this one anymore, maybe should, if at all, be merged right at the next release since it adds a public variable, even if it is a private header?

[PATCH 2/2] 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.8 6.7 6.5 6.2 5.15
Change-Id: Iec180bdfaaf913a3a1560210c781966dc99c0d42
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit d8bd4c2306f2acfefc75f8163b58f2037596dc65)

asturmlechner 2024-10-16: Resolve conflict with dev branch commit
   b2c236792c6eaa676b2dce3d8abb4f5948957699

[PATCH 1/2] QThread: initialize interruptionRequested atomic

Coverity complains correctly that the variable is not initialized.
std::atomic's default constructor doesn't do it, and we only do it
in QThread::start and QThread::finished. So initialize it using NSMDI.

Pick-to: 6.8 6.7 6.5
Coverity-Id: 466429
Change-Id: I36d4624d46718f50e10bec17ef5e437c60541fa9
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit d1f6d2258809ed7e2d7aa50de3b6d6f472cf1bd8)

Merge request reports