From b15f069996610481a6ae8aaca9b174504ac68135 Mon Sep 17 00:00:00 2001
From: Arttu Tarkiainen <arttu.tarkiainen@qt.io>
Date: Wed, 31 Aug 2022 12:44:40 +0300
Subject: Windows: fix installation error with concurrent Extract operations

There was an issue with the Extract operations when using the same
"targetDir" argument for multiple archives. The DirectoryGuard object
creating the missing leading directories would occasionally run into
a race condition when multiple threads competed with creating the
directories.

Fix by adding installer specific implementation similar to
QDir::mkpath(), but which checks if the directory was created
elsewhere at every directory level. Also convert an existing similar
case with the Extract operation to use the new function.

Task-number: QTIFW-2752
Change-Id: I4451e931309edb536294314b11c903189dacf2f0
Reviewed-by: Katja Marttila <katja.marttila@qt.io>
---
 .../installer/extractarchiveoperationtest/data.qrc |   1 +
 .../extractarchiveoperationtest/data/subdirs.7z    | Bin 0 -> 18715 bytes
 .../tst_extractarchiveoperationtest.cpp            |  52 +++++++++++++++++++++
 3 files changed, 53 insertions(+)
 create mode 100644 tests/auto/installer/extractarchiveoperationtest/data/subdirs.7z

(limited to 'tests/auto')

diff --git a/tests/auto/installer/extractarchiveoperationtest/data.qrc b/tests/auto/installer/extractarchiveoperationtest/data.qrc
index 974c0c6f7..a8cc1a892 100644
--- a/tests/auto/installer/extractarchiveoperationtest/data.qrc
+++ b/tests/auto/installer/extractarchiveoperationtest/data.qrc
@@ -2,6 +2,7 @@
     <qresource prefix="/">
         <file>data/valid.7z</file>
         <file>data/invalid.7z</file>
+        <file>data/subdirs.7z</file>
         <file>data/xmloperationrepository/Updates.xml</file>
         <file>data/xmloperationrepository/A/1.0.0content.7z</file>
         <file>data/xmloperationrepository/A/1.0.0content1.tar.gz</file>
diff --git a/tests/auto/installer/extractarchiveoperationtest/data/subdirs.7z b/tests/auto/installer/extractarchiveoperationtest/data/subdirs.7z
new file mode 100644
index 000000000..7f93fe106
Binary files /dev/null and b/tests/auto/installer/extractarchiveoperationtest/data/subdirs.7z differ
diff --git a/tests/auto/installer/extractarchiveoperationtest/tst_extractarchiveoperationtest.cpp b/tests/auto/installer/extractarchiveoperationtest/tst_extractarchiveoperationtest.cpp
index 8ceaa76c7..89492bd94 100644
--- a/tests/auto/installer/extractarchiveoperationtest/tst_extractarchiveoperationtest.cpp
+++ b/tests/auto/installer/extractarchiveoperationtest/tst_extractarchiveoperationtest.cpp
@@ -28,6 +28,7 @@
 
 #include "../shared/packagemanager.h"
 
+#include "concurrentoperationrunner.h"
 #include "init.h"
 #include "extractarchiveoperation.h"
 
@@ -86,6 +87,57 @@ private slots:
         QCOMPARE(UpdateOperation::Error(op.error()), UpdateOperation::UserDefinedError);
     }
 
+    void testConcurrentExtractWithCompetingData()
+    {
+        // Suppress warnings about already deleted installerResources file
+        qInstallMessageHandler(silentTestMessageHandler);
+
+        const QString testDirectory = generateTemporaryFileName()
+            + "/subdir1/subdir2/subdir3/subdir4/subdir5/";
+
+        QStringList created7zList;
+
+        OperationList operations;
+        for (int i = 0; i < 100; ++i) {
+            ExtractArchiveOperation *op = new ExtractArchiveOperation(nullptr);
+            // We add the same data multiple times, and extract to same directory.
+            // Can't open the same archive multiple times however so it needs to
+            // be copied to unique files.
+            const QString new7zPath = generateTemporaryFileName() + ".7z";
+            QFile old7z(":///data/subdirs.7z");
+            QVERIFY(old7z.copy(new7zPath));
+
+            op->setArguments(QStringList() << new7zPath << testDirectory);
+            operations.append(op);
+        }
+        ConcurrentOperationRunner runner(&operations, Operation::Backup);
+
+        const QHash<Operation *, bool> backupResults = runner.run();
+        const OperationList backupOperations = backupResults.keys();
+
+        for (auto *operation : backupOperations)
+            QVERIFY2((backupResults.value(operation) && operation->error() == Operation::NoError),
+                     operation->errorString().toLatin1());
+
+        runner.setType(Operation::Perform);
+        const QHash<Operation *, bool> results = runner.run();
+        const OperationList performedOperations = results.keys();
+
+        for (auto *operation : performedOperations)
+            QVERIFY2((results.value(operation) && operation->error() == Operation::NoError),
+                     operation->errorString().toLatin1());
+
+        for (auto *operation : operations)
+            QVERIFY(operation->undoOperation());
+
+        qDeleteAll(operations);
+
+        for (const QString &archive : created7zList)
+            QFile::remove(archive);
+
+        QDir().rmdir(testDirectory);
+    }
+
     void testExtractArchiveFromXML()
     {
         m_testDirectory = QInstaller::generateTemporaryFileName();
-- 
cgit v1.2.3