QThread/Unix: split the destruction of the exiting thread's QThreadData

As noted in the comment, we had a chicken-and-the-egg problem with
plugins and the event dispatchers: we need to destroy the event
dispatcher when plugins are still loaded (otherwise we could and did
crash) but we also wanted the QThreadData to exist when the plugins
unload.

The solution for that is to split the work: destroy the event dispatcher
first, then unload pugins, then destroy the current thread's
QThreadData. On ELF systems, this is guaranteed to work because we set
the init_priority to call destroy_current_thread_data() to higher than
QLibraryStore. In other systems, it's a best effort with dynamic
libraries and not guaranteed at all with static builds (don't even
report bugs).

Fixes: QTBUG-134080
Fixes: QTBUG-133861
Task-number: QTBUG-132697
Task-number: QTBUG-102984
Task-number: QTBUG-132381
Pick-to: 6.9 6.9.0 6.8
Change-Id: Ifaa28bb87338f4117d51fffdf721da68c0762e5a
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

src/corelib/thread/qthread_unix.cpp

@@ -123,21 +123,38 @@ int pthread_timedjoin_np(...) { return ENOSYS; }    // pretend
 // https://github.com/llvm/llvm-project/blob/llvmorg-19.1.0/libcxxabi/src/cxa_thread_atexit.cpp#L118-L120
 #endif
 //
-// However, we can't destroy the QThreadData for the thread that called
-// ::exit() that early, because a lot of existing content (including in Qt)
-// runs when the static destructors are run and they do depend on QThreadData
-// being extant. Likewise, we can't destroy it at global static destructor time
-// because it's too late: the event dispatcher is usually a class found in a
-// plugin and the plugin's destructors (as well as QtGui's) will have run. So
-// we strike a middle-ground and destroy at function-local static destructor
-// time (see set_thread_data()), because those run after the thread_local ones,
-// before the global ones, and in reverse order of creation.
+// Thus, the destruction of QThreadData is split into 3 steps:
+// - finish()
+// - cleanup()
+// - deref & delete
+//
+// The reason for the first split is that user content may run as a result of
+// the finished() signal, in thread_local destructors or similar, so we don't
+// want to destroy the event dispatcher too soon. If we did, the event
+// dispatcher could get recreated.
+//
+// For auxiliary threads started with QThread, finish() is run as soon as run()
+// returns, while cleanup() and the deref happen at pthread_set_specific
+// destruction time (except for broken_threadlocal_dtors, see above).
+//
+// For auxiliary threads started with something else and adopted as a
+// QAdoptedThread, there's only one choice: all three steps happen at at
+// pthread_set_specific destruction time.
+//
+// Finally, for the thread that called ::exit() (which in most cases happens by
+// returning from the main() function), finish() and cleanup() happen at
+// function-local static destructor time, and the deref & delete happens later,
+// at global static destruction time. This is important so QLibraryStore can
+// unload plugins between those two steps: it is also destroyed by a global
+// static destructor, but with a lower priority than ours. The order needs to
+// be this way so we delete the event dispatcher before plugins unload, as
+// often the dispatcher for the main thread is provided by a QPA plugin, but
+// the QThreadData object must still be alive when the plugins do unload.
 
 Q_CONSTINIT static thread_local QThreadData *currentThreadData = nullptr;
 
-static void destroy_current_thread_data(void *p)
+static void destroy_current_thread_data(QThreadData *data)
 {
-    QThreadData *data = static_cast<QThreadData *>(p);
     QThread *thread = data->thread.loadAcquire();
 
 #ifdef Q_OS_APPLE
@@ -164,7 +181,10 @@ static void destroy_current_thread_data(void *p)
         // We may be racing the QThread destructor in another thread and it may
         // have begun destruction; we must not dereference the QThread pointer.
     }
+}
 
+static void deref_current_thread_data(QThreadData *data)
+{
     // the QThread object may still have a reference, so this may not delete
     data->deref();
 
@@ -173,6 +193,13 @@ static void destroy_current_thread_data(void *p)
     currentThreadData = nullptr;
 }
 
+static void destroy_auxiliary_thread_data(void *p)
+{
+    auto data = static_cast<QThreadData *>(p);
+    destroy_current_thread_data(data);
+    deref_current_thread_data(data);
+}
+
 // Utility functions for getting, setting and clearing thread specific data.
 static QThreadData *get_thread_data()
 {
@@ -180,17 +207,35 @@ static QThreadData *get_thread_data()
 }
 
 namespace {
-struct PThreadTlsKey
+struct QThreadDataDestroyer
 {
     pthread_key_t key;
-    PThreadTlsKey() noexcept { pthread_key_create(&key, destroy_current_thread_data); }
-    ~PThreadTlsKey() { pthread_key_delete(key); }
+    QThreadDataDestroyer() noexcept
+    {
+        pthread_key_create(&key, &destroy_auxiliary_thread_data);
+    }
+    ~QThreadDataDestroyer()
+    {
+        // running global static destructors upon ::exit()
+        if (QThreadData *data = get_thread_data())
+            deref_current_thread_data(data);
+        pthread_key_delete(key);
+    }
+
+    struct EarlyMainThread {
+        ~EarlyMainThread()
+        {
+            // running function-local destructors upon ::exit()
+            if (QThreadData *data = get_thread_data())
+                destroy_current_thread_data(data);
+        }
+    };
 };
 }
 #if QT_SUPPORTS_INIT_PRIORITY
 Q_DECL_INIT_PRIORITY(10)
 #endif
-static PThreadTlsKey pthreadTlsKey; // intentional non-trivial init & destruction
+static QThreadDataDestroyer threadDataDestroyer; // intentional non-trivial init & destruction
 
 static void set_thread_data(QThreadData *data) noexcept
 {
@@ -198,13 +243,8 @@ static void set_thread_data(QThreadData *data) noexcept
         // As noted above: one global static for the thread that called
         // ::exit() (which may not be a Qt thread) and the pthread_key_t for
         // all others.
-        static struct Cleanup {
-            ~Cleanup() {
-                if (QThreadData *data = get_thread_data())
-                    destroy_current_thread_data(data);
-            }
-        } currentThreadCleanup;
-        pthread_setspecific(pthreadTlsKey.key, data);
+        QThreadDataDestroyer::EarlyMainThread currentThreadCleanup;
+        pthread_setspecific(threadDataDestroyer.key, data);
     }
     currentThreadData = data;
 }

tests/auto/corelib/kernel/qcoreapplication/CMakeLists.txt

@@ -60,3 +60,15 @@ set_target_properties(apphelper_core PROPERTIES OUTPUT_NAME apphelper)
 add_dependencies(tst_qcoreapplication
     apphelper_core
 )
+
+if(QT_FEATURE_library)
+    qt_internal_add_cmake_library(apphelper_core_plugin
+        MODULE
+        SOURCES
+            apphelper_plugin.cpp
+        LIBRARIES
+            Qt::Core
+    )
+    qt_autogen_tools_initial_setup(apphelper_core_plugin)
+    add_dependencies(apphelper_core apphelper_core_plugin)
+endif()

tests/auto/corelib/kernel/qcoreapplication/apphelper.cpp

@@ -5,6 +5,10 @@
 #include <qthread.h>
 #include <qtimer.h>
 
+#if QT_CONFIG(library)
+#  include <qpluginloader.h>
+#endif
+
 #include <string_view>
 #include <thread>
 
@@ -89,6 +93,45 @@ static int exitFromThreadedEventLoop(int argc, char **argv)
     Q_UNREACHABLE_RETURN(EXIT_FAILURE);
 }
 
+// see QTBUG-134080
+static int exitWithPlugins(int argc, char **argv)
+{
+#if defined(Q_OS_QNX)
+    // The plugin loading fails: "The shared library was not found".
+    puts("Plugin doesn't get deployed.");
+    return -1;
+#elif defined(Q_OS_WIN)
+    puts("Not possible on Windows: DLL destruction order does not follow C++ Standard.");
+    return -1;
+#elif QT_CONFIG(library)
+    TestApplication app(argc, argv);
+
+    QString pluginName = app.applicationDirPath() + "/apphelper_"
+#if defined(QT_WIDGETS_LIB)
+            "widgets"
+#elif defined(QT_GUI_LIB)
+            "gui"
+#else
+            "core"
+#endif
+            "_plugin";
+
+    QPluginLoader loader(pluginName);
+    QObject *instance = loader.instance();
+    if (!instance) {
+        fprintf(stderr, "Did not get an instance from the plugin: %s\n",
+                qPrintable(loader.errorString()));
+    }
+
+    [[maybe_unused]] auto w = maybeShowSomething();
+    QTimer::singleShot(200, &app, &TestApplication::quit);
+    return app.exec();
+#else
+    puts("Qt built without plugin support")
+    return -1;
+#endif
+}
+
 // see QTBUG-130895
 static int mainAppInAThread(int argc, char **argv)
 {
@@ -168,6 +211,8 @@ int main(int argc, char **argv)
         return exitFromThread(argc - 1, argv + 1);
     if (subtest == "exitFromThreadedEventLoop")
         return exitFromThreadedEventLoop(argc - 1, argv + 1);
+    if (subtest == "exitWithPlugins")
+        return exitWithPlugins(argc - 1, argv + 1);
     if (subtest == "mainAppInAThread")
         return mainAppInAThread(argc - 1, argv + 1);
 

tests/auto/corelib/kernel/qcoreapplication/apphelper_plugin.cpp

@@ -0,0 +1,42 @@
+// Copyright (C) 2025 Intel Corporation.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only
+
+#ifdef Q_ASSERT
+#  error "Don't use precompiled headers"
+#endif
+#define QT_FORCE_ASSERTS
+
+#include <QObject>
+#include <QThread>
+#include <QtPlugin>
+
+class Interface
+{
+public:
+    virtual ~Interface() {}
+};
+
+#define Interface_iid "org.qt-project.Qt.autotests.plugininterface"
+QT_BEGIN_NAMESPACE
+Q_DECLARE_INTERFACE(Interface, Interface_iid)
+QT_END_NAMESPACE
+
+class ApplicationHelperPlugin : public QObject, public Interface
+{
+    Q_OBJECT
+    Q_PLUGIN_METADATA(IID Interface_iid)
+    Q_INTERFACES(Interface)
+
+public:
+    ApplicationHelperPlugin()
+    {
+    }
+    ~ApplicationHelperPlugin()
+    {
+        // see QTBUG-134080
+        // We used to print "Timers cannot be stopped from another thread"
+        Q_ASSERT(thread() == QThread::currentThread());
+    }
+};
+
+#include "apphelper_plugin.moc"

tests/auto/corelib/kernel/qcoreapplication/tst_qcoreapplication.h

@@ -53,6 +53,7 @@ private slots:
     void exitFromEventLoop() { QCoreApplicationTestHelper::run(); }
     void exitFromThread() { QCoreApplicationTestHelper::run(); }
     void exitFromThreadedEventLoop() { QCoreApplicationTestHelper::run(); }
+    void exitWithPlugins() { QCoreApplicationTestHelper::run(); }
     void mainAppInAThread() { QCoreApplicationTestHelper::run(); }
 
     void testTrWithPercantegeAtTheEnd();

tests/auto/corelib/kernel/qcoreapplication/tst_static_qcoreapplication.cpp

@@ -69,11 +69,10 @@ struct HookManager
 #if !defined(QT_GUI_LIB) && !defined(Q_OS_WIN)
         // Only tested for QtCore/QCoreApplication on Unix. QtGui has statics
         // with QObject that haven't been cleaned up.
-        if (int c = objectCount.loadRelaxed())
+        // For QtCore, we expect exactly one object: the QAdoptedThread for
+        // represents the main thread.
+        if (int c = objectCount.loadRelaxed(); c > 1)
             qFatal("%d objects still alive", c);
-
-        if (void *id = QCoreApplicationPrivate::theMainThreadId.loadRelaxed())
-            qFatal("QCoreApplicationPrivate::theMainThreadId is still set - %p", id);
 #endif
     }
 };

tests/auto/gui/kernel/qguiapplication/CMakeLists.txt

@@ -70,3 +70,15 @@ set_target_properties(apphelper_gui PROPERTIES OUTPUT_NAME apphelper)
 add_dependencies(tst_qguiapplication
     apphelper_gui
 )
+
+if(QT_FEATURE_library)
+    qt_internal_add_cmake_library(apphelper_gui_plugin
+        MODULE
+        SOURCES
+            ../../../corelib/kernel/qcoreapplication/apphelper_plugin.cpp
+        LIBRARIES
+            Qt::Gui
+    )
+    qt_autogen_tools_initial_setup(apphelper_gui_plugin)
+    add_dependencies(apphelper_gui apphelper_gui_plugin)
+endif()

tests/auto/widgets/kernel/qapplication/CMakeLists.txt

@@ -22,6 +22,18 @@ qt_internal_add_executable(apphelper_widgets
 )
 set_target_properties(apphelper_widgets PROPERTIES OUTPUT_NAME apphelper)
 
+if(QT_FEATURE_library)
+    qt_internal_add_cmake_library(apphelper_widgets_plugin
+        MODULE
+        SOURCES
+            ../../../corelib/kernel/qcoreapplication/apphelper_plugin.cpp
+        LIBRARIES
+            Qt::Widgets
+    )
+    qt_autogen_tools_initial_setup(apphelper_widgets_plugin)
+    add_dependencies(apphelper_widgets apphelper_widgets_plugin)
+endif()
+
 add_dependencies(tst_qapplication
     desktopsettingsaware_helper
     modal_helper

tests/auto/widgets/kernel/qapplication/tst_qapplication.cpp

@@ -97,6 +97,7 @@ private slots:
     void exitFromEventLoop() { QCoreApplicationTestHelper::run(); }
     void exitFromThread() { QCoreApplicationTestHelper::run(); }
     void exitFromThreadedEventLoop() { QCoreApplicationTestHelper::run(); }
+    void exitWithPlugins() { QCoreApplicationTestHelper::run(); }
     void mainAppInAThread() { QCoreApplicationTestHelper::run(); }
 
     void thread();