Fix crash when calling QQmlEngine::clearComponentCache()

We must protect various resources in the type loader with our existing
lock. The QQmlTypeLoaderQmldirContent is now value based, so that we can
release the lock on the shared cache early. Copying it involves
adjusting the refcount of the QHash and QString instances in the
QQmlDirParser.

The safety of this was verified with a TSAN build and the example
supplied in the task. It crashed reliably with TASN errors first and
with this patch it runs without errors.

Task-number: QTBUG-41465
Change-Id: I616843c4b8bdfd65d1277d4faa8cb884d8e77df8
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

1 files changed, 30 insertions, 25 deletions

diff --git a/src/qml/qml/qqmltypeloader.cpp b/src/qml/qml/qqmltypeloader.cpp
index 5b954605e0..2b778b0b63 100644
--- a/src/qml/qml/qqmltypeloader.cpp
+++ b/src/qml/qml/qqmltypeloader.cpp
@@ -1370,8 +1370,8 @@ bool QQmlTypeLoader::Blob::updateQmldir(QQmlQmldirData *data, const QV4::Compile
     if (!importQualifier.isEmpty()) {
         // Does this library contain any qualified scripts?
         QUrl libraryUrl(qmldirUrl);
-        const QQmlTypeLoaderQmldirContent *qmldir = typeLoader()->qmldirContent(qmldirIdentifier);
-        const auto qmldirScripts = qmldir->scripts();
+        const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirIdentifier);
+        const auto qmldirScripts = qmldir.scripts();
         for (const QQmlDirParser::Script &script : qmldirScripts) {
             QUrl scriptUrl = libraryUrl.resolved(QUrl(script.fileName));
             QQmlScriptBlob *blob = typeLoader()->getScript(scriptUrl);
@@ -1418,8 +1418,8 @@ bool QQmlTypeLoader::Blob::addImport(const QV4::CompiledData::Import *import, QL
             if (!importQualifier.isEmpty()) {
                 // Does this library contain any qualified scripts?
                 QUrl libraryUrl(qmldirUrl);
-                const QQmlTypeLoaderQmldirContent *qmldir = typeLoader()->qmldirContent(qmldirFilePath);
-                const auto qmldirScripts = qmldir->scripts();
+                const QQmlTypeLoaderQmldirContent qmldir = typeLoader()->qmldirContent(qmldirFilePath);
+                const auto qmldirScripts = qmldir.scripts();
                 for (const QQmlDirParser::Script &script : qmldirScripts) {
                     QUrl scriptUrl = libraryUrl.resolved(QUrl(script.fileName));
                     QQmlScriptBlob *blob = typeLoader()->getScript(scriptUrl);
@@ -1584,6 +1584,7 @@ QString QQmlTypeLoaderQmldirContent::typeNamespace() const
 
 void QQmlTypeLoaderQmldirContent::setContent(const QString &location, const QString &content)
 {
+    m_hasContent = true;
     m_location = location;
     m_parser.parse(content);
 }
@@ -1808,6 +1809,7 @@ QString QQmlTypeLoader::absoluteFilePath(const QString &path)
     int lastSlash = path.lastIndexOf(QLatin1Char('/'));
     QString dirPath(path.left(lastSlash));
 
+    LockHolder<QQmlTypeLoader> holder(this);
     if (!m_importDirCache.contains(dirPath)) {
         bool exists = QDir(dirPath).exists();
         QCache<QString, bool> *entry = exists ? new QCache<QString, bool> : nullptr;
@@ -1871,6 +1873,7 @@ bool QQmlTypeLoader::directoryExists(const QString &path)
         --length;
     QString dirPath(path.left(length));
 
+    LockHolder<QQmlTypeLoader> holder(this);
     if (!m_importDirCache.contains(dirPath)) {
         bool exists = QDir(dirPath).exists();
         QCache<QString, bool> *files = exists ? new QCache<QString, bool> : nullptr;
@@ -1889,8 +1892,10 @@ Return a QQmlTypeLoaderQmldirContent for absoluteFilePath.  The QQmlTypeLoaderQm
 
 It can also be a remote path for a remote directory import, but it will have been cached by now in this case.
 */
-const QQmlTypeLoaderQmldirContent *QQmlTypeLoader::qmldirContent(const QString &filePathIn)
+const QQmlTypeLoaderQmldirContent QQmlTypeLoader::qmldirContent(const QString &filePathIn)
 {
+    LockHolder<QQmlTypeLoader> holder(this);
+
     QString filePath;
 
     // Try to guess if filePathIn is already a URL. This is necessarily fragile, because
@@ -1904,39 +1909,39 @@ const QQmlTypeLoaderQmldirContent *QQmlTypeLoader::qmldirContent(const QString &
         filePath = filePathIn;
     } else {
         filePath = QQmlFile::urlToLocalFileOrQrc(url);
-        if (filePath.isEmpty()) // Can't load the remote here, but should be cached
-            return *(m_importQmlDirCache.value(filePathIn));
+        if (filePath.isEmpty()) { // Can't load the remote here, but should be cached
+            if (auto entry = m_importQmlDirCache.value(filePathIn))
+                return **entry;
+            else
+                return QQmlTypeLoaderQmldirContent();
+        }
     }
 
-    QQmlTypeLoaderQmldirContent *qmldir;
     QQmlTypeLoaderQmldirContent **val = m_importQmlDirCache.value(filePath);
-    if (!val) {
-        qmldir = new QQmlTypeLoaderQmldirContent;
+    if (val)
+        return **val;
+    QQmlTypeLoaderQmldirContent *qmldir = new QQmlTypeLoaderQmldirContent;
 
 #define ERROR(description) { QQmlError e; e.setDescription(description); qmldir->setError(e); }
 #define NOT_READABLE_ERROR QString(QLatin1String("module \"$$URI$$\" definition \"%1\" not readable"))
 #define CASE_MISMATCH_ERROR QString(QLatin1String("cannot load module \"$$URI$$\": File name case mismatch for \"%1\""))
 
-        QFile file(filePath);
-        if (!QQml_isFileCaseCorrect(filePath)) {
-            ERROR(CASE_MISMATCH_ERROR.arg(filePath));
-        } else if (file.open(QFile::ReadOnly)) {
-            QByteArray data = file.readAll();
-            qmldir->setContent(filePath, QString::fromUtf8(data));
-        } else {
-            ERROR(NOT_READABLE_ERROR.arg(filePath));
-        }
+    QFile file(filePath);
+    if (!QQml_isFileCaseCorrect(filePath)) {
+        ERROR(CASE_MISMATCH_ERROR.arg(filePath));
+    } else if (file.open(QFile::ReadOnly)) {
+        QByteArray data = file.readAll();
+        qmldir->setContent(filePath, QString::fromUtf8(data));
+    } else {
+        ERROR(NOT_READABLE_ERROR.arg(filePath));
+    }
 
 #undef ERROR
 #undef NOT_READABLE_ERROR
 #undef CASE_MISMATCH_ERROR
 
-        m_importQmlDirCache.insert(filePath, qmldir);
-    } else {
-        qmldir = *val;
-    }
-
-    return qmldir;
+    m_importQmlDirCache.insert(filePath, qmldir);
+    return *qmldir;
 }
 
 void QQmlTypeLoader::setQmldirContent(const QString &url, const QString &content)