QThread/Unix: fix normal exit/terminate() race

The QThreadPrivate::finish() cleanup code is not prepared to be
subjected to POSIX cancellation: if, like on glibc, thread
cancellation is implemented by stack ununwinding, we get exceptions in
dtors and therefore std::terminate(), and otherwise, we leak
resources.

It would be very hard to make the code robust against this, as it
would require all cleanup to be wrapped in pthread_cleanup_push/pop,
with the added problem that these functions need to appear in the same
lexical scope. Another alternative would be to move all cleanup code
into a thread_local destructor, but it's not clear whether code
running as part of thread_local destruction would be exempt from
cancellation, and it would be a major rewrite.

The simplest method is to disable cancellation for the remainder of
the thread lifetime in the shutdown code, just like the startup code
only enables cancellation after initial setup, so do that.

[ChangeLog][Important Behavior Changes][QThread] On Unix,
fixed a race of QThread::terminate() with normal thread exit (running
off the end of run()) which could corrupt QThread's internal cleanup
code. The fix involves disabling thread cancellation for the remainder
of the thread's lifetime once control reaches QThread's cleanup
code. If you rely on a PTHREAD_CANCELED return status, be aware that
this change may mask late cancellations. Likewise, slots connected to
QThread::finished() using Qt::DirectConnection are now run in a regime
where thread cancellation is already disabled. If you need
cancellation in that situation to work, you need to define your own
finished()-like signal and emit that at the end of run().

Fixes: QTBUG-127008
Change-Id: I23030eefdfcebf0a6d6796db5cbbbf0812ae12c0
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 01d4be4a8327458a3242804594b498f840480289)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
This commit is contained in:
Marc Mutz 2024-07-08 18:25:42 +02:00 committed by Qt Cherry-pick Bot
parent 9f331f89ea
commit 894837577d
2 changed files with 7 additions and 1 deletions

View File

@ -345,6 +345,13 @@ void QThreadPrivate::finish(void *arg)
QThread *thr = reinterpret_cast<QThread *>(arg);
QThreadPrivate *d = thr->d_func();
// Disable cancellation; we're already in the finishing touches of this
// thread, and we don't want cleanup to be disturbed by
// abi::__forced_unwind being thrown from all kinds of functions.
#ifdef PTHREAD_CANCEL_DISABLE
pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, nullptr);
#endif
QMutexLocker locker(&d->mutex);
d->isInFinish = true;

View File

@ -1951,7 +1951,6 @@ void tst_QThread::terminateSelfStressTest()
struct Thread : QThread {
void run() override {
terminate();
while (true) sleep(1ns); // QTBUG-127008
}
};