From d09c6c1046ef008851af9dc698cab49f1e779c9b Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Sun, 9 Feb 2025 11:13:19 -0800 Subject: [PATCH] Short live QCoreApplication::instanceExists() This is a thread-safe version of QCoreApplication::instance() != nullptr for Qt 6.x, because QCoreApplication::self is not atomic and thus reading it from outside the main thread could be a data race. That's not to say it always is: if by construction the code can only run in the main thread or while QCoreApplication definitely exists, that's safe. Therefore, this commit focuses on places that are meant to be used in multi-threaded environment (ruling out most of QtGui and QtWidgets) or where the code was going to dereference the returned pointer anyway. Change-Id: I6fc556c5fe5cbe0b5902fffdfb6b3bb345b0ee50 Reviewed-by: Marc Mutz --- src/corelib/global/qlibraryinfo.cpp | 10 +++++----- src/corelib/kernel/qcoreapplication.cpp | 22 ++++++++++++++++------ src/corelib/kernel/qcoreapplication.h | 2 ++ src/corelib/kernel/qcoreapplication_p.h | 1 - src/corelib/kernel/qcoreevent.cpp | 2 +- src/corelib/kernel/qeventloop.cpp | 2 +- src/corelib/kernel/qtranslator.cpp | 2 +- src/corelib/thread/qthreadstorage.cpp | 2 +- src/dbus/qdbusintegrator.cpp | 2 +- src/gui/image/qpixmap.cpp | 2 +- src/network/kernel/qdnslookup.cpp | 2 +- src/sql/kernel/qsqldatabase.cpp | 6 +++--- 12 files changed, 33 insertions(+), 22 deletions(-) diff --git a/src/corelib/global/qlibraryinfo.cpp b/src/corelib/global/qlibraryinfo.cpp index 4b116c54b2e..da39f1eb6c2 100644 --- a/src/corelib/global/qlibraryinfo.cpp +++ b/src/corelib/global/qlibraryinfo.cpp @@ -62,14 +62,14 @@ QLibrarySettings::QLibrarySettings() : paths(false), reloadOnQAppAvailable(false QSettings *QLibrarySettings::configuration() { - if (reloadOnQAppAvailable && QCoreApplication::instance() != nullptr) + if (reloadOnQAppAvailable && QCoreApplication::instanceExists()) load(); return settings.get(); } bool QLibrarySettings::havePaths() { - if (reloadOnQAppAvailable && QCoreApplication::instance() != nullptr) + if (reloadOnQAppAvailable && QCoreApplication::instanceExists()) load(); return paths; } @@ -78,7 +78,7 @@ void QLibrarySettings::load() { // If we get any settings here, those won't change when the application shows up. settings = findConfiguration(); - reloadOnQAppAvailable = !settings && !QCoreApplication::instance(); + reloadOnQAppAvailable = !settings && !QCoreApplication::instanceExists(); if (settings) { // This code needs to be in the regular library, as otherwise a qt.conf that @@ -121,7 +121,7 @@ static std::unique_ptr findConfiguration() } } #endif - if (QCoreApplication::instance()) { + if (QCoreApplication::instanceExists()) { QString pwd = QCoreApplication::applicationDirPath(); qtconfig = pwd + u"/qt" QT_STRINGIFY(QT_VERSION_MAJOR) ".conf"_s; if (QFile::exists(qtconfig)) @@ -270,7 +270,7 @@ QVersionNumber QLibraryInfo::version() noexcept static QString prefixFromAppDirHelper() { - if (QCoreApplication::instance()) { + if (QCoreApplication::instanceExists()) { #ifdef Q_OS_DARWIN CFBundleRef bundleRef = CFBundleGetMainBundle(); if (bundleRef) { diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index 93ecf12b758..5008ed579dd 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -144,6 +144,21 @@ Q_CONSTINIT QCoreApplication *QCoreApplication::self = nullptr; Q_CONSTINIT static QBasicAtomicPointer g_self = nullptr; # undef qApp # define qApp g_self.loadRelaxed() + +/*! + \internal + + This function is a Qt 6 thread-safe (no data races) version of: + \code + QCoreApplication::instance() != nullptr + \endcode + + We may remove it in Qt 7.0 because the above will be thread-safe. +*/ +bool QCoreApplication::instanceExists() noexcept +{ + return qApp != nullptr; +} #endif #if !defined(Q_OS_WIN) || defined(QT_BOOTSTRAPPED) @@ -276,7 +291,7 @@ void qAddPreRoutine(QtStartUpFunction p) return; if (preRoutinesCalled) { - Q_ASSERT(QCoreApplication::instance()); + Q_ASSERT(qApp); p(); } @@ -467,11 +482,6 @@ QCoreApplicationPrivate::~QCoreApplicationPrivate() #endif } -bool QCoreApplicationPrivate::isAlive() noexcept -{ - return qApp != nullptr; -} - #ifndef QT_NO_QOBJECT void QCoreApplicationPrivate::cleanupThreadData() diff --git a/src/corelib/kernel/qcoreapplication.h b/src/corelib/kernel/qcoreapplication.h index 87e78035d60..ac9de4db5e7 100644 --- a/src/corelib/kernel/qcoreapplication.h +++ b/src/corelib/kernel/qcoreapplication.h @@ -96,8 +96,10 @@ public: #if QT_VERSION >= QT_VERSION_CHECK(7, 0, 0) static QCoreApplication *instance() noexcept { return self.loadRelaxed(); } + static bool instanceExists() noexcept { return instance() != nullptr; } #else static QCoreApplication *instance() noexcept { return self; } + static bool instanceExists() noexcept; #endif #ifndef QT_NO_QOBJECT diff --git a/src/corelib/kernel/qcoreapplication_p.h b/src/corelib/kernel/qcoreapplication_p.h index 021f1a25387..98a5af29167 100644 --- a/src/corelib/kernel/qcoreapplication_p.h +++ b/src/corelib/kernel/qcoreapplication_p.h @@ -61,7 +61,6 @@ public: #endif ~QCoreApplicationPrivate(); - static bool isAlive() noexcept; void init(); QString appName() const; diff --git a/src/corelib/kernel/qcoreevent.cpp b/src/corelib/kernel/qcoreevent.cpp index d638e3524ca..8b6e7c83262 100644 --- a/src/corelib/kernel/qcoreevent.cpp +++ b/src/corelib/kernel/qcoreevent.cpp @@ -329,7 +329,7 @@ QEvent::QEvent(Type type) QEvent::~QEvent() { - if (m_posted && QCoreApplication::instance()) + if (m_posted && QCoreApplication::instanceExists()) QCoreApplicationPrivate::removePostedEvent(this); } diff --git a/src/corelib/kernel/qeventloop.cpp b/src/corelib/kernel/qeventloop.cpp index c02afd172dc..a07589addb6 100644 --- a/src/corelib/kernel/qeventloop.cpp +++ b/src/corelib/kernel/qeventloop.cpp @@ -68,7 +68,7 @@ QEventLoop::QEventLoop(QObject *parent) { Q_D(QEventLoop); QThreadData *threadData = d->threadData.loadRelaxed(); - if (!QCoreApplication::instance() && threadData->requiresCoreApplication) { + if (!QCoreApplication::instanceExists() && threadData->requiresCoreApplication) { qWarning("QEventLoop: Cannot be used without QCoreApplication"); } else { threadData->ensureEventDispatcher(); diff --git a/src/corelib/kernel/qtranslator.cpp b/src/corelib/kernel/qtranslator.cpp index 713c029c347..fe7d0b2f21e 100644 --- a/src/corelib/kernel/qtranslator.cpp +++ b/src/corelib/kernel/qtranslator.cpp @@ -397,7 +397,7 @@ QTranslator::QTranslator(QObject * parent) QTranslator::~QTranslator() { - if (QCoreApplication::instance()) + if (QCoreApplication::instanceExists()) QCoreApplication::removeTranslator(this); Q_D(QTranslator); d->clear(); diff --git a/src/corelib/thread/qthreadstorage.cpp b/src/corelib/thread/qthreadstorage.cpp index 3b2d4f5c9ca..0deff1b19d9 100644 --- a/src/corelib/thread/qthreadstorage.cpp +++ b/src/corelib/thread/qthreadstorage.cpp @@ -157,7 +157,7 @@ void QThreadStorageData::finish(void **p) locker.unlock(); if (!destructor) { - if (QCoreApplicationPrivate::isAlive()) + if (QCoreApplication::instanceExists()) qWarning("QThreadStorage: entry %d destroyed before end of thread %p", i, QThread::currentThread()); continue; diff --git a/src/dbus/qdbusintegrator.cpp b/src/dbus/qdbusintegrator.cpp index 534999378d5..bdd49464b50 100644 --- a/src/dbus/qdbusintegrator.cpp +++ b/src/dbus/qdbusintegrator.cpp @@ -559,7 +559,7 @@ bool QDBusConnectionPrivate::handleMessage(const QDBusMessage &amsg) // run it through the spy filters (if any) before the regular processing: // a) if it's a local message, we're in the caller's thread, so invoke the filter directly // b) if it's an external message, post to the main thread - if (Q_UNLIKELY(qDBusSpyHookList.exists()) && qApp) { + if (Q_UNLIKELY(qDBusSpyHookList.exists()) && QCoreApplication::instanceExists()) { if (isLocal) { Q_ASSERT(QThread::currentThread() != thread()); qDBusDebug() << this << "invoking message spies directly"; diff --git a/src/gui/image/qpixmap.cpp b/src/gui/image/qpixmap.cpp index cbfb5e2d8d4..a0ba3ccef51 100644 --- a/src/gui/image/qpixmap.cpp +++ b/src/gui/image/qpixmap.cpp @@ -48,7 +48,7 @@ QT_WARNING_DISABLE_MSVC(4723) static bool qt_pixmap_thread_test() { - if (Q_UNLIKELY(!QCoreApplication::instance())) { + if (!QCoreApplication::instanceExists()) { qFatal("QPixmap: Must construct a QGuiApplication before a QPixmap"); return false; } diff --git a/src/network/kernel/qdnslookup.cpp b/src/network/kernel/qdnslookup.cpp index 182540c37f2..9151eb84e4f 100644 --- a/src/network/kernel/qdnslookup.cpp +++ b/src/network/kernel/qdnslookup.cpp @@ -776,7 +776,7 @@ void QDnsLookup::lookup() Q_D(QDnsLookup); d->isFinished = false; d->reply = QDnsLookupReply(); - if (!QCoreApplication::instance()) { + if (!QCoreApplication::instanceExists()) { // NOT qCWarning because this isn't a result of the lookup qWarning("QDnsLookup requires a QCoreApplication"); return; diff --git a/src/sql/kernel/qsqldatabase.cpp b/src/sql/kernel/qsqldatabase.cpp index bb7d5a1ea53..70840010555 100644 --- a/src/sql/kernel/qsqldatabase.cpp +++ b/src/sql/kernel/qsqldatabase.cpp @@ -23,12 +23,12 @@ Q_STATIC_LOGGING_CATEGORY(lcSqlDb, "qt.sql.qsqldatabase") using namespace Qt::StringLiterals; #define CHECK_QCOREAPPLICATION \ - if (Q_UNLIKELY(!QCoreApplication::instance())) { \ + if (Q_UNLIKELY(!QCoreApplication::instanceExists())) { \ qCWarning(lcSqlDb, "QSqlDatabase requires a QCoreApplication"); \ return; \ } #define CHECK_QCOREAPPLICATION_RETVAL \ - if (Q_UNLIKELY(!QCoreApplication::instance())) { \ + if (Q_UNLIKELY(!QCoreApplication::instanceExists())) { \ qCWarning(lcSqlDb, "QSqlDatabase requires a QCoreApplication"); \ return {}; \ } @@ -661,7 +661,7 @@ void QSqlDatabasePrivate::init(const QString &type) qCWarning(lcSqlDb, "QSqlDatabase: %ls driver not loaded", qUtf16Printable(type)); qCWarning(lcSqlDb, "QSqlDatabase: available drivers: %ls", qUtf16Printable(QSqlDatabase::drivers().join(u' '))); - if (QCoreApplication::instance() == nullptr) + if (!QCoreApplication::instanceExists()) qCWarning(lcSqlDb, "QSqlDatabase: an instance of QCoreApplication is required for loading driver plugins"); driver = shared_null()->driver; }