From 76c3eee4020cae681857ee17406c655f61f9082c Mon Sep 17 00:00:00 2001 From: Mike Achtelik Date: Wed, 17 Jun 2020 08:04:03 +0200 Subject: [PATCH] Remove pthread storage for thread local data The thread specific QThreadData is currently referenced as a thread_local variable and using a thread specific value via pthread_setspecific. Having both is not necessary, as the pthread value is never directly accessed and only used in the pthread destructor. Using a holder, we can achieve the same and get rid of the pthread handling altogether. This also fixes a bug, where the thread_local currentThreadData is already null, when entering the pthread destructor. In this case it would lead to a new QThreadData being created, when finishing an adopted thread. Pick-to: 5.15 Change-Id: Ib23b840f804637e6b7cebd89016672a34a575380 Reviewed-by: Thiago Macieira --- src/corelib/thread/qthread_unix.cpp | 82 ++++++++--------------------- 1 file changed, 22 insertions(+), 60 deletions(-) diff --git a/src/corelib/thread/qthread_unix.cpp b/src/corelib/thread/qthread_unix.cpp index 35bfd35f1a3..18b6caf5fc9 100644 --- a/src/corelib/thread/qthread_unix.cpp +++ b/src/corelib/thread/qthread_unix.cpp @@ -109,81 +109,43 @@ static_assert(sizeof(pthread_t) <= sizeof(Qt::HANDLE)); enum { ThreadPriorityResetFlag = 0x80000000 }; -static thread_local QThreadData *currentThreadData = nullptr; - -static pthread_once_t current_thread_data_once = PTHREAD_ONCE_INIT; -static pthread_key_t current_thread_data_key; - -static void destroy_current_thread_data(void *p) +class QThreadDataHolder { -#if defined(Q_OS_VXWORKS) - // Calling setspecific(..., 0) sets the value to 0 for ALL threads. - // The 'set to 1' workaround adds a bit of an overhead though, - // since this function is called twice now. - if (p == (void *)1) - return; -#endif - // POSIX says the value in our key is set to zero before calling - // this destructor function, so we need to set it back to the - // right value... - pthread_setspecific(current_thread_data_key, p); - QThreadData *data = static_cast(p); - if (data->isAdopted) { - QThread *thread = data->thread.loadAcquire(); - Q_ASSERT(thread); - QThreadPrivate *thread_p = static_cast(QObjectPrivate::get(thread)); - Q_ASSERT(!thread_p->finished); - thread_p->finish(thread); +public: + QThreadData* data; + + ~QThreadDataHolder() + { + if (!data) + return; + + if (data->isAdopted) { + QThread *thread = data->thread.loadAcquire(); + Q_ASSERT(thread); + QThreadPrivate *thread_p = static_cast(QObjectPrivate::get(thread)); + Q_ASSERT(!thread_p->finished); + thread_p->finish(thread); + } + data->deref(); } - data->deref(); - - // ... but we must reset it to zero before returning so we aren't - // called again (POSIX allows implementations to call destructor - // functions repeatedly until all values are zero) - pthread_setspecific(current_thread_data_key, -#if defined(Q_OS_VXWORKS) - (void *)1); -#else - nullptr); -#endif -} - -static void create_current_thread_data_key() -{ - pthread_key_create(¤t_thread_data_key, destroy_current_thread_data); -} - -static void destroy_current_thread_data_key() -{ - pthread_once(¤t_thread_data_once, create_current_thread_data_key); - pthread_key_delete(current_thread_data_key); - - // Reset current_thread_data_once in case we end up recreating - // the thread-data in the rare case of QObject construction - // after destroying the QThreadData. - pthread_once_t pthread_once_init = PTHREAD_ONCE_INIT; - current_thread_data_once = pthread_once_init; -} -Q_DESTRUCTOR_FUNCTION(destroy_current_thread_data_key) +}; +static thread_local QThreadDataHolder currentThreadData; // Utility functions for getting, setting and clearing thread specific data. static QThreadData *get_thread_data() { - return currentThreadData; + return currentThreadData.data; } static void set_thread_data(QThreadData *data) { - currentThreadData = data; - pthread_once(¤t_thread_data_once, create_current_thread_data_key); - pthread_setspecific(current_thread_data_key, data); + currentThreadData.data = data; } static void clear_thread_data() { - currentThreadData = nullptr; - pthread_setspecific(current_thread_data_key, nullptr); + currentThreadData.data = nullptr; } template