Skip to content

Commit

Permalink
QThread::terminate(): don't depend on stack unwinding
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
marcmutz committed Jul 18, 2024
1 parent 6f70ab0 commit 272c021
Showing 1 changed file with 14 additions and 3 deletions.
17 changes: 14 additions & 3 deletions src/corelib/thread/qthread_unix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -751,16 +751,27 @@ 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)
return;

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");
}
Expand Down

0 comments on commit 272c021

Please sign in to comment.