From 06904a7a772357479a1b78b8345716f8d0711b4c Mon Sep 17 00:00:00 2001 From: Aram So Date: Mon, 14 Nov 2016 16:42:32 +0900 Subject: Make calling QCoreApplication::translate() thread-safe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed crash on QCoreApplication::translate() call from qqmlThread while QCoreApplication::{install,remove}Translator() is called from the GUI thread. [ChangeLog][QtCore][QCoreApplication] Calling QCoreApplication::translate() is now thread-safe. Task-number: QTBUG-57095 Change-Id: Ie5340a42040a829f311c01332e05d4bbaf60462c Reviewed-by: Olivier Goffart (Woboq GmbH) Reviewed-by: Tor Arne Vestbø --- src/corelib/kernel/qcoreapplication.cpp | 44 +++++++++++--------- src/corelib/kernel/qcoreapplication_p.h | 2 +- .../corelib/kernel/qtranslator/tst_qtranslator.cpp | 47 ++++++++++++++++++++++ 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/src/corelib/kernel/qcoreapplication.cpp b/src/corelib/kernel/qcoreapplication.cpp index a5bd2513b6..a08021be96 100644 --- a/src/corelib/kernel/qcoreapplication.cpp +++ b/src/corelib/kernel/qcoreapplication.cpp @@ -1931,7 +1931,10 @@ bool QCoreApplication::installTranslator(QTranslator *translationFile) if (!QCoreApplicationPrivate::checkInstance("installTranslator")) return false; QCoreApplicationPrivate *d = self->d_func(); - d->translators.prepend(translationFile); + { + QWriteLocker locker(&d->translateMutex); + d->translators.prepend(translationFile); + } #ifndef QT_NO_TRANSLATION_BUILDER if (translationFile->isEmpty()) @@ -1963,8 +1966,10 @@ bool QCoreApplication::removeTranslator(QTranslator *translationFile) if (!QCoreApplicationPrivate::checkInstance("removeTranslator")) return false; QCoreApplicationPrivate *d = self->d_func(); + QWriteLocker locker(&d->translateMutex); if (d->translators.removeAll(translationFile)) { #ifndef QT_NO_QOBJECT + locker.unlock(); if (!self->closingDown()) { QEvent ev(QEvent::LanguageChange); QCoreApplication::sendEvent(self, &ev); @@ -2000,7 +2005,7 @@ static void replacePercentN(QString *result, int n) } /*! - \reentrant + \threadsafe Returns the translation text for \a sourceText, by querying the installed translation files. The translation files are searched @@ -2029,13 +2034,7 @@ static void replacePercentN(QString *result, int n) This function is not virtual. You can use alternative translation techniques by subclassing \l QTranslator. - \warning This method is reentrant only if all translators are - installed \e before calling this method. Installing or removing - translators while performing translations is not supported. Doing - so will most likely result in crashes or other undesirable - behavior. - - \sa QObject::tr(), installTranslator() + \sa QObject::tr(), installTranslator(), removeTranslator(), translate(), isTranslatorInstalled() */ QString QCoreApplication::translate(const char *context, const char *sourceText, const char *disambiguation, int n) @@ -2045,14 +2044,18 @@ QString QCoreApplication::translate(const char *context, const char *sourceText, if (!sourceText) return result; - if (self && !self->d_func()->translators.isEmpty()) { - QList::ConstIterator it; - QTranslator *translationFile; - for (it = self->d_func()->translators.constBegin(); it != self->d_func()->translators.constEnd(); ++it) { - translationFile = *it; - result = translationFile->translate(context, sourceText, disambiguation, n); - if (!result.isNull()) - break; + if (self) { + QCoreApplicationPrivate *d = self->d_func(); + QReadLocker locker(&d->translateMutex); + if (!d->translators.isEmpty()) { + QList::ConstIterator it; + QTranslator *translationFile; + for (it = d->translators.constBegin(); it != d->translators.constEnd(); ++it) { + translationFile = *it; + result = translationFile->translate(context, sourceText, disambiguation, n); + if (!result.isNull()) + break; + } } } @@ -2076,8 +2079,11 @@ QString qtTrId(const char *id, int n) bool QCoreApplicationPrivate::isTranslatorInstalled(QTranslator *translator) { - return QCoreApplication::self - && QCoreApplication::self->d_func()->translators.contains(translator); + if (!QCoreApplication::self) + return false; + QCoreApplicationPrivate *d = QCoreApplication::self->d_func(); + QReadLocker locker(&d->translateMutex); + return d->translators.contains(translator); } #else diff --git a/src/corelib/kernel/qcoreapplication_p.h b/src/corelib/kernel/qcoreapplication_p.h index da6ce1249f..963aec70e8 100644 --- a/src/corelib/kernel/qcoreapplication_p.h +++ b/src/corelib/kernel/qcoreapplication_p.h @@ -142,7 +142,7 @@ public: #ifndef QT_NO_TRANSLATION QTranslatorList translators; - + QReadWriteLock translateMutex; static bool isTranslatorInstalled(QTranslator *translator); #endif diff --git a/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp b/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp index 66971af7b4..5bfe133966 100644 --- a/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp +++ b/tests/auto/corelib/kernel/qtranslator/tst_qtranslator.cpp @@ -50,6 +50,7 @@ private slots: void loadFromResource(); void loadDirectory(); void dependencies(); + void translationInThreadWhileInstallingTranslator(); private: int languageChangeEventCounter; @@ -287,6 +288,52 @@ void tst_QTranslator::dependencies() } } +struct TranslateThread : public QThread +{ + bool ok = false; + QAtomicInt terminate; + QMutex startupLock; + QWaitCondition runningCondition; + + void run() { + bool startSignalled = false; + + while (terminate.load() == 0) { + const QString result = QCoreApplication::translate("QPushButton", "Hello %n world(s)!", 0, 0); + + if (!startSignalled) { + QMutexLocker startupLocker(&startupLock); + runningCondition.wakeAll(); + startSignalled = true; + } + + ok = (result == QLatin1String("Hallo 0 Welten!")) + || (result == QLatin1String("Hello 0 world(s)!")); + if (!ok) + break; + } + } +}; + +void tst_QTranslator::translationInThreadWhileInstallingTranslator() +{ + TranslateThread thread; + + QMutexLocker startupLocker(&thread.startupLock); + + thread.start(); + + thread.runningCondition.wait(&thread.startupLock); + + QTranslator *tor = new QTranslator; + tor->load("hellotr_la"); + QCoreApplication::installTranslator(tor); + + ++thread.terminate; + + QVERIFY(thread.wait()); + QVERIFY(thread.ok); +} QTEST_MAIN(tst_QTranslator) #include "tst_qtranslator.moc" -- cgit v1.2.3