From 38f77d09e89c93cedf6da4144fc8aafe95975891 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 14 Nov 2024 15:10:06 -0700 Subject: [PATCH] QThreadData: refactor current() now that it never returns null Simplying the body of the function to rely less on the exception handling too. The implementations for Unix and Windows are now literally identical, but duplicated because they call different static functions Change-Id: Ia14910a1c778ff5e606afffdabb8035e4603edda Reviewed-by: Thiago Macieira Reviewed-by: Fabian Kosmale --- src/corelib/thread/qthread.cpp | 4 +-- src/corelib/thread/qthread_p.h | 2 +- src/corelib/thread/qthread_unix.cpp | 45 +++++++++++++---------------- src/corelib/thread/qthread_win.cpp | 39 ++++++++++++++----------- 4 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/corelib/thread/qthread.cpp b/src/corelib/thread/qthread.cpp index 0574386dacd..f804ab8b9d3 100644 --- a/src/corelib/thread/qthread.cpp +++ b/src/corelib/thread/qthread.cpp @@ -1119,9 +1119,9 @@ void QThread::setTerminationEnabled(bool) // No threads: so we can just use static variables Q_CONSTINIT static QThreadData *data = nullptr; -QThreadData *QThreadData::current(bool createIfNecessary) +QThreadData *QThreadData::current() { - if (!data && createIfNecessary) { + if (!data) { data = new QThreadData; data->thread = new QAdoptedThread(data); } diff --git a/src/corelib/thread/qthread_p.h b/src/corelib/thread/qthread_p.h index 0871f9847df..51299ee53b9 100644 --- a/src/corelib/thread/qthread_p.h +++ b/src/corelib/thread/qthread_p.h @@ -308,7 +308,7 @@ public: } ~QThreadData(); - static Q_AUTOTEST_EXPORT QThreadData *current(bool createIfNecessary = true); + static Q_AUTOTEST_EXPORT QThreadData *current(); static void clearCurrentThreadData(); static QThreadData *get2(QThread *thread) { Q_ASSERT_X(thread != nullptr, "QThread", "internal error"); return thread->d_func()->data; } diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp index caca492be72..832a21db48c 100644 --- a/src/corelib/thread/qthread_unix.cpp +++ b/src/corelib/thread/qthread_unix.cpp @@ -124,7 +124,6 @@ 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 // QT_CONFIG(broken_threadlocal_dtors) -// Always access this through the {get,set,clear}_thread_data() functions. Q_CONSTINIT static thread_local QThreadData *currentThreadData = nullptr; static void destroy_current_thread_data(void *p) @@ -176,13 +175,13 @@ static void destroy_main_thread_data() Q_DESTRUCTOR_FUNCTION(destroy_main_thread_data) #endif -static void set_thread_data(QThreadData *data) +static void set_thread_data(QThreadData *data) noexcept { if (data) { if constexpr (QT_CONFIG(broken_threadlocal_dtors)) { static pthread_key_t tls_key; struct TlsKey { - TlsKey() { pthread_key_create(&tls_key, destroy_current_thread_data); } + TlsKey() noexcept { pthread_key_create(&tls_key, destroy_current_thread_data); } ~TlsKey() { pthread_key_delete(tls_key); } }; static TlsKey currentThreadCleanup; @@ -197,11 +196,6 @@ static void set_thread_data(QThreadData *data) currentThreadData = data; } -static void clear_thread_data() -{ - set_thread_data(nullptr); -} - template static typename std::enable_if, Qt::HANDLE>::type to_HANDLE(T id) { @@ -228,27 +222,28 @@ static typename std::enable_if, T>::type from_HANDLE(Qt::HA void QThreadData::clearCurrentThreadData() { - clear_thread_data(); + set_thread_data(nullptr); } -QThreadData *QThreadData::current(bool createIfNecessary) +QThreadData *QThreadData::current() { - QThreadData *data = get_thread_data(); - if (!data && createIfNecessary) { - data = new QThreadData; - QT_TRY { - set_thread_data(data); - data->thread.storeRelease(new QAdoptedThread(data)); - } QT_CATCH(...) { - clear_thread_data(); - data->deref(); - data = nullptr; - QT_RETHROW; - } - } - return data; -} + if (QThreadData *data = get_thread_data(); Q_LIKELY(data)) + return data; + std::unique_ptr data = std::make_unique(); + + // This needs to be called prior to new QAdoptedThread() to avoid + // recursion (see qobject.cpp). + set_thread_data(data.get()); + + QT_TRY { + data->thread.storeRelease(new QAdoptedThread(data.get())); + } QT_CATCH(...) { + clearCurrentThreadData(); + QT_RETHROW; + } + return data.release(); +} void QAdoptedThread::init() { diff --git a/src/corelib/thread/qthread_win.cpp b/src/corelib/thread/qthread_win.cpp index 94b4e41133a..947c1a4e475 100644 --- a/src/corelib/thread/qthread_win.cpp +++ b/src/corelib/thread/qthread_win.cpp @@ -72,7 +72,12 @@ static void destroy_current_thread_data(void *p) currentThreadData = nullptr; } -static void set_thread_data(QThreadData *data) +static QThreadData *get_thread_data() +{ + return currentThreadData; +} + +static void set_thread_data(QThreadData *data) noexcept { if (data) { struct Cleanup { @@ -91,24 +96,24 @@ void QThreadData::clearCurrentThreadData() set_thread_data(nullptr); } -QThreadData *QThreadData::current(bool createIfNecessary) +QThreadData *QThreadData::current() { - QThreadData *threadData = currentThreadData; - if (!threadData && createIfNecessary) { - threadData = new QThreadData; - // This needs to be called prior to new AdoptedThread() to - // avoid recursion. - set_thread_data(threadData); - QT_TRY { - threadData->thread.storeRelease(new QAdoptedThread(threadData)); - } QT_CATCH(...) { - set_thread_data(nullptr); - threadData->deref(); - threadData = 0; - QT_RETHROW; - } + if (QThreadData *data = get_thread_data(); Q_LIKELY(data)) + return data; + + std::unique_ptr data = std::make_unique(); + + // This needs to be called prior to new QAdoptedThread() to avoid + // recursion (see qobject.cpp). + set_thread_data(data.get()); + + QT_TRY { + data->thread.storeRelease(new QAdoptedThread(data.get())); + } QT_CATCH(...) { + clearCurrentThreadData(); + QT_RETHROW; } - return threadData; + return data.release(); } void QAdoptedThread::init()