From caa5358fe97becafeba9631f4c4d2fc69469afb1 Mon Sep 17 00:00:00 2001 From: Andy Shaw Date: Wed, 26 Apr 2017 00:16:16 +0200 Subject: Fix concurrent loading of the same qmldir from different scripts QQmlQmldirData keeps a pointer to a QQmlScript::Import, and an integer priority. Each Blob that is waiting on it was setting its own import and priority even though the QQmlQmldirData itself was shared. This resulted in whichever one began loading last succeeding to load, and the rest failing. This change instead stores the import and priority data per-dependent Blob Fix was originally done by Josh Faust . I added the test. Task-number: QTBUG-30469 Change-Id: Id3d15569a999a7c22eeb12b431e5daf1ddae51dc Reviewed-by: Simon Hausmann --- src/qml/qml/qqmltypeloader.cpp | 41 +++++++++++++--------- src/qml/qml/qqmltypeloader_p.h | 12 +++---- .../auto/qml/qqmllanguage/data/ConcurrentLoadA.qml | 7 ++++ .../auto/qml/qqmllanguage/data/ConcurrentLoadB.qml | 7 ++++ .../qml/qqmllanguage/data/concurrentLoad_main.qml | 5 +++ tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp | 16 +++++++++ 6 files changed, 65 insertions(+), 23 deletions(-) create mode 100644 tests/auto/qml/qqmllanguage/data/ConcurrentLoadA.qml create mode 100644 tests/auto/qml/qqmllanguage/data/ConcurrentLoadB.qml create mode 100644 tests/auto/qml/qqmllanguage/data/concurrentLoad_main.qml diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp index 40bd2e5020..e4293596d8 100644 --- a/src/qml/qml/qqmltypeloader.cpp +++ b/src/qml/qml/qqmltypeloader.cpp @@ -1320,8 +1320,8 @@ bool QQmlTypeLoader::Blob::fetchQmldir(const QUrl &url, const QV4::CompiledData: { QQmlQmldirData *data = typeLoader()->getQmldir(url); - data->setImport(import); - data->setPriority(priority); + data->setImport(this, import); + data->setPriority(this, priority); if (data->status() == Error) { // This qmldir must not exist - which is not an error @@ -1349,7 +1349,7 @@ bool QQmlTypeLoader::Blob::updateQmldir(QQmlQmldirData *data, const QV4::Compile QHash::iterator it = m_unresolvedImports.find(import); if (it != m_unresolvedImports.end()) { - *it = data->priority(); + *it = data->priority(this); } // Release this reference at destruction @@ -1482,7 +1482,7 @@ void QQmlTypeLoader::Blob::dependencyComplete(QQmlDataBlob *blob) if (blob->type() == QQmlDataBlob::QmldirFile) { QQmlQmldirData *data = static_cast(blob); - const QV4::CompiledData::Import *import = data->import(); + const QV4::CompiledData::Import *import = data->import(this); QList errors; if (!qmldirDataAvailable(data, &errors)) { @@ -1506,11 +1506,11 @@ bool QQmlTypeLoader::Blob::qmldirDataAvailable(QQmlQmldirData *data, QListimport(); - data->setImport(0); + const QV4::CompiledData::Import *import = data->import(this); + data->setImport(this, 0); - int priority = data->priority(); - data->setPriority(0); + int priority = data->priority(this); + data->setPriority(this, 0); if (import) { // Do we need to resolve this import? @@ -3069,7 +3069,7 @@ void QQmlScriptBlob::initializeFromCompilationUnit(QV4::CompiledData::Compilatio } QQmlQmldirData::QQmlQmldirData(const QUrl &url, QQmlTypeLoader *loader) -: QQmlTypeLoader::Blob(url, QmldirFile, loader), m_import(0), m_priority(0) +: QQmlTypeLoader::Blob(url, QmldirFile, loader) { } @@ -3078,24 +3078,31 @@ const QString &QQmlQmldirData::content() const return m_content; } -const QV4::CompiledData::Import *QQmlQmldirData::import() const +const QV4::CompiledData::Import *QQmlQmldirData::import(QQmlTypeLoader::Blob *blob) const { - return m_import; + QHash::const_iterator it = + m_imports.find(blob); + if (it == m_imports.end()) + return 0; + return *it; } -void QQmlQmldirData::setImport(const QV4::CompiledData::Import *import) +void QQmlQmldirData::setImport(QQmlTypeLoader::Blob *blob, const QV4::CompiledData::Import *import) { - m_import = import; + m_imports[blob] = import; } -int QQmlQmldirData::priority() const +int QQmlQmldirData::priority(QQmlTypeLoader::Blob *blob) const { - return m_priority; + QHash::const_iterator it = m_priorities.find(blob); + if (it == m_priorities.end()) + return 0; + return *it; } -void QQmlQmldirData::setPriority(int priority) +void QQmlQmldirData::setPriority(QQmlTypeLoader::Blob *blob, int priority) { - m_priority = priority; + m_priorities[blob] = priority; } void QQmlQmldirData::dataReceived(const SourceCodeData &data) diff --git a/src/qml/qml/qqmltypeloader_p.h b/src/qml/qml/qqmltypeloader_p.h index 48e7d5cba4..f367fa6f58 100644 --- a/src/qml/qml/qqmltypeloader_p.h +++ b/src/qml/qml/qqmltypeloader_p.h @@ -572,11 +572,11 @@ private: public: const QString &content() const; - const QV4::CompiledData::Import *import() const; - void setImport(const QV4::CompiledData::Import *); + const QV4::CompiledData::Import *import(QQmlTypeLoader::Blob *) const; + void setImport(QQmlTypeLoader::Blob *, const QV4::CompiledData::Import *); - int priority() const; - void setPriority(int); + int priority(QQmlTypeLoader::Blob *) const; + void setPriority(QQmlTypeLoader::Blob *, int); protected: void dataReceived(const SourceCodeData &) override; @@ -584,8 +584,8 @@ protected: private: QString m_content; - const QV4::CompiledData::Import *m_import; - int m_priority; + QHash m_imports; + QHash m_priorities; }; diff --git a/tests/auto/qml/qqmllanguage/data/ConcurrentLoadA.qml b/tests/auto/qml/qqmllanguage/data/ConcurrentLoadA.qml new file mode 100644 index 0000000000..3d538c7572 --- /dev/null +++ b/tests/auto/qml/qqmllanguage/data/ConcurrentLoadA.qml @@ -0,0 +1,7 @@ +import QtQuick 2.0 +import testModule 1.0 + +Item { + Test {} +} + diff --git a/tests/auto/qml/qqmllanguage/data/ConcurrentLoadB.qml b/tests/auto/qml/qqmllanguage/data/ConcurrentLoadB.qml new file mode 100644 index 0000000000..3d538c7572 --- /dev/null +++ b/tests/auto/qml/qqmllanguage/data/ConcurrentLoadB.qml @@ -0,0 +1,7 @@ +import QtQuick 2.0 +import testModule 1.0 + +Item { + Test {} +} + diff --git a/tests/auto/qml/qqmllanguage/data/concurrentLoad_main.qml b/tests/auto/qml/qqmllanguage/data/concurrentLoad_main.qml new file mode 100644 index 0000000000..8cfc90ac96 --- /dev/null +++ b/tests/auto/qml/qqmllanguage/data/concurrentLoad_main.qml @@ -0,0 +1,5 @@ +import QtQuick 2.0 +Rectangle { +ConcurrentLoadA {} +ConcurrentLoadB {} +} diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp index 750c32cc3c..39f5082c70 100644 --- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp +++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp @@ -263,6 +263,8 @@ private slots: void qmlTypeCanBeResolvedByName_data(); void qmlTypeCanBeResolvedByName(); + void concurrentLoadQmlDir(); + private: QQmlEngine engine; QStringList defaultImportPathList; @@ -4338,6 +4340,20 @@ void tst_qqmllanguage::qmlTypeCanBeResolvedByName() QVERIFY(!o.isNull()); } +void tst_qqmllanguage::concurrentLoadQmlDir() +{ + ThreadedTestHTTPServer server(dataDirectory()); + QString serverdir = server.urlString("/lib/"); + engine.setImportPathList(QStringList(defaultImportPathList) << serverdir); + + QQmlComponent component(&engine, testFileUrl("concurrentLoad_main.qml")); + QTRY_VERIFY(component.isReady()); + VERIFY_ERRORS(0); + QScopedPointer o(component.create()); + QVERIFY(!o.isNull()); + engine.setImportPathList(defaultImportPathList); +} + QTEST_MAIN(tst_qqmllanguage) #include "tst_qqmllanguage.moc" -- cgit v1.2.3