QThread/Unix: implement joining of the launched thread, if we can

This is only implemented for OSes that provide a way to perform a timed
join. For all other OSes, we stick to the previous implementation, which
as the comment indicates, may run for arbitrarily long time after wait()
has returned, running user code (e.g., pthread_setspecific() and
thread_local destructors).

Instead, if we perform the joining, we are assured by pthread and the OS
that the thread has exited and no user code remains running.

Unfortunately, this only applies to non-adopted threads, because we
can't pthread_join() a thread we didn't start.

As of this writing, this code only applies to Linux/glibc. MUSL, Bionic
and several BSDs have pthread_timedjoin, but that takes a CLOCK_REALTIME
absolute time, which means it's subject to time jumps, while
QWaitCondition can sometimes use the monotonic clock in those systems.

Change-Id: I692e24d7411742447e10fffd650fe84f6a9cdedd
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

src/corelib/configure.cmake

@@ -432,6 +432,20 @@ poll(&pfd, 1, 0);
 }
 ")
 
+# pthread_clockjoin
+qt_config_compile_test(pthread_clockjoin
+    LABEL "pthread_clockjoin()"
+    LIBRARIES Threads::Threads
+    CODE
+"#include <pthread.h>
+int main()
+{
+    void *ret;
+    const struct timespec ts = {};
+    return pthread_clockjoin_np(pthread_self(), &ret, CLOCK_MONOTONIC, &ts);
+}
+")
+
 # renameat2
 qt_config_compile_test(renameat2
     LABEL "renameat2()"
@@ -694,6 +708,11 @@ qt_feature("posix_shm" PRIVATE
     LABEL "POSIX shared memory"
     CONDITION TEST_posix_shm AND UNIX
 )
+qt_feature("pthread_clockjoin" PRIVATE
+    LABEL "pthread_clockjoin() function"
+    AUTODETECT UNIX
+    CONDITION UNIX AND QT_FEATURE_thread AND TEST_pthread_clockjoin
+)
 qt_feature("qqnx_pps" PRIVATE
     LABEL "PPS"
     CONDITION PPS_FOUND

src/corelib/thread/qthread.cpp

@@ -182,7 +182,6 @@ QThreadPrivate::QThreadPrivate(QThreadData *d)
 #if defined (Q_OS_WIN)
     handle = 0;
     id = 0;
-    waiters = 0;
     terminationEnabled = true;
     terminatePending = false;
 #endif

src/corelib/thread/qthread_p.h

@@ -190,6 +190,7 @@ public:
         Running = 1,        // in run()
         Finishing = 2,      // in QThreadPrivate::finish()
         Finished = 3,       // QThreadPrivate::finish() or cleanup() is done
+                            //    or, if using pthread_join, joining is done
     };
 
     State threadState = NotStarted;
@@ -199,6 +200,7 @@ public:
     bool terminated = false; // when (the first) terminate has been called
 #endif
 
+    int waiters = 0;
     int returnCode = -1;
 
     uint stackSize = 0;
@@ -215,6 +217,7 @@ public:
 #ifdef Q_OS_UNIX
     QWaitCondition thread_done;
 
+    void wakeAll();
     static void *start(void *arg);
     void finish();          // happens early (before thread-local dtors)
     void cleanup();         // happens late (as a thread-local dtor, if possible)
@@ -226,7 +229,6 @@ public:
 
     Qt::HANDLE handle;
     unsigned int id;
-    int waiters;
     bool terminationEnabled, terminatePending;
 #endif // Q_OS_WIN
 #ifdef Q_OS_WASM

src/corelib/thread/qthread_unix.cpp

@@ -73,6 +73,30 @@ static_assert(sizeof(pthread_t) <= sizeof(Qt::HANDLE));
 
 enum { ThreadPriorityResetFlag = 0x80000000 };
 
+#if QT_CONFIG(pthread_clockjoin)
+// If we have a way to perform a timed pthread_join(), we will do it. This
+// ensures that QThread::wait() only returns after pthread_join() or equivalent
+// has returned, ensuring that the thread has definitely exited.
+//
+// Because only one thread can call this family of functions at a time, we
+// count how many threads are waiting and all but one of them wait on a
+// QWaitCondition, with the joining thread having the responsibility for waking
+// up all others when the joining concludes. If the joining times out, the
+// thread in charge wakes up one of the other waiters (if there's any) to
+// assume responsibility for joining.
+//
+// We don't bother with pthread_timedjoin() implementations that take a
+// CLOCK_REALTIME timeout.
+#else
+// If we don't have a way to perform timed pthread_join(), then we don't try
+// joining a all. All waiting threads will wait for the launched thread to
+// call QWaitCondition::wakeAll(). Note in this case it is possible for the
+// waiting threads to conclude the launched thread has exited before it has.
+//
+// To support this scenario, we start the thread in detached state.
+int pthread_clockjoin_np(...) { return ENOSYS; }    // pretend
+#endif
+
 #if QT_CONFIG(broken_threadlocal_dtors)
 // On most modern platforms, the C runtime has a helper function that helps the
 // C++ runtime run the thread_local non-trivial destructors when threads exit
@@ -411,12 +435,9 @@ void QThreadPrivate::cleanup()
             locker.relock();
         }
 
-        d->threadState = QThreadPrivate::Finished;
         d->interruptionRequested.store(false, std::memory_order_relaxed);
 
-        d->data->threadId.storeRelaxed(nullptr);
-
-        d->thread_done.wakeAll();
+        d->wakeAll();
     });
 }
 
@@ -673,7 +694,7 @@ void QThread::start(Priority priority)
     QMutexLocker locker(&d->mutex);
 
     if (d->threadState == QThreadPrivate::Finishing)
-        d->thread_done.wait(locker.mutex());
+        d->wait(locker, QDeadlineTimer::Forever);
 
     if (d->threadState == QThreadPrivate::Running)
         return;
@@ -686,7 +707,8 @@ void QThread::start(Priority priority)
 
     pthread_attr_t attr;
     pthread_attr_init(&attr);
-    pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
+    if constexpr (!QT_CONFIG(pthread_clockjoin))
+        pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_DETACHED);
 #ifdef Q_OS_DARWIN
     if (d->serviceLevel != QThread::QualityOfService::Auto)
         pthread_attr_set_qos_class_np(&attr, d->nativeQualityOfServiceClass(), 0);
@@ -820,6 +842,20 @@ void QThread::terminate()
 #endif
 }
 
+static void wakeAllInternal(QThreadPrivate *d)
+{
+    d->threadState = QThreadPrivate::Finished;
+    d->data->threadId.storeRelaxed(nullptr);
+    if (d->waiters)
+        d->thread_done.wakeAll();
+}
+
+inline void QThreadPrivate::wakeAll()
+{
+    if (data->isAdopted || !QT_CONFIG(pthread_clockjoin))
+        wakeAllInternal(this);
+}
+
 bool QThread::wait(QDeadlineTimer deadline)
 {
     Q_D(QThread);
@@ -840,17 +876,72 @@ bool QThread::wait(QDeadlineTimer deadline)
 
 bool QThreadPrivate::wait(QMutexLocker<QMutex> &locker, QDeadlineTimer deadline)
 {
+    constexpr int HasJoinerBit = int(0x8000'0000);  // a.k.a. sign bit
+    struct timespec ts, *pts = nullptr;
+    if (!deadline.isForever()) {
+        ts = deadlineToAbstime(deadline);
+        pts = &ts;
+    }
+
+    auto doJoin = [&] {
+        // pthread_join() & family are cancellation points
+        struct CancelState {
+            QThreadPrivate *d;
+            QMutexLocker<QMutex> *locker;
+            int joinResult = ETIMEDOUT;
+            static void run(void *arg) { static_cast<CancelState *>(arg)->run(); }
+            void run()
+            {
+                locker->relock();
+                if (joinResult == ETIMEDOUT && d->waiters)
+                    d->thread_done.wakeOne();
+                else if (joinResult == 0)
+                    wakeAllInternal(d);
+                d->waiters &= ~HasJoinerBit;
+            }
+        } nocancel = { this, &locker };
+        int &r = nocancel.joinResult;
+
+        // we're going to perform the join, so don't let other threads do it
+        waiters |= HasJoinerBit;
+        locker.unlock();
+
+        pthread_cleanup_push(&CancelState::run, &nocancel);
+        pthread_t thrId = from_HANDLE<pthread_t>(data->threadId.loadRelaxed());
+        r = pthread_clockjoin_np(thrId, nullptr, SteadyClockClockId, pts);
+        Q_ASSERT(r == 0 || r == ETIMEDOUT);
+        pthread_cleanup_pop(1);
+
+        Q_ASSERT(waiters >= 0);
+        return r != ETIMEDOUT;
+    };
     Q_ASSERT(threadState != QThreadPrivate::Finished);
     Q_ASSERT(locker.isLocked());
-    QThreadPrivate *d = this;
 
-    while (d->threadState != QThreadPrivate::Finished) {
-        if (!d->thread_done.wait(locker.mutex(), deadline))
-            return false;
+    bool result = false;
+
+    // both branches call cancellation points
+    ++waiters;
+    bool mustJoin = (waiters & HasJoinerBit) == 0;
+    pthread_cleanup_push([](void *ptr) {
+        --(*static_cast<decltype(waiters) *>(ptr));
+    }, &waiters);
+    for (;;) {
+        if (QT_CONFIG(pthread_clockjoin) && mustJoin && !data->isAdopted) {
+            result = doJoin();
+            break;
+        }
+        if (!thread_done.wait(locker.mutex(), deadline))
+            break;      // timed out
+        result = threadState == QThreadPrivate::Finished;
+        if (result)
+            break;      // success
+        mustJoin = (waiters & HasJoinerBit) == 0;
     }
-    Q_ASSERT(d->data->threadId.loadRelaxed() == nullptr);
+    pthread_cleanup_pop(1);
 
-    return true;
+    Q_ASSERT(!result || data->threadId.loadRelaxed() == nullptr);
+    return result;
 }
 
 void QThread::setTerminationEnabled(bool enabled)

tests/auto/corelib/thread/qthread/tst_qthread.cpp

@@ -1220,6 +1220,12 @@ void tst_QThread::multiThreadWait_data()
         QTest::newRow(name.join('-').constData()) << deadlines;
     };
 
+    // control
+    addRow(-1);
+    addRow(0);
+    addRow(25);
+    addRow(250);
+
     addRow(0, 0);
     addRow(0, 0, 0, 0, 0);
     addRow(-1, -1);