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>

4 files changed, 50 insertions, 16 deletions

diff --git a/src/libs/installer/directoryguard.cpp b/src/libs/installer/directoryguard.cpp
index 9c97130a4..014d213d7 100644
--- a/src/libs/installer/directoryguard.cpp
+++ b/src/libs/installer/directoryguard.cpp
@@ -28,6 +28,7 @@
 
 #include "directoryguard.h"
 
+#include "fileutils.h"
 #include "globals.h"
 #include "errors.h"
 
@@ -92,8 +93,7 @@ QStringList DirectoryGuard::tryCreate()
         toCreate = QDir(p);
     }
 
-    QDir dir(m_path);
-    m_created = dir.mkpath(m_path);
+    m_created = QInstaller::createDirectoryWithParents(m_path);
     if (!m_created) {
         throw Error(QCoreApplication::translate("DirectoryGuard",
             "Cannot create directory \"%1\".").arg(QDir::toNativeSeparators(m_path)));
diff --git a/src/libs/installer/extractarchiveoperation.cpp b/src/libs/installer/extractarchiveoperation.cpp
index 6b9ecb687..986b9d8c8 100644
--- a/src/libs/installer/extractarchiveoperation.cpp
+++ b/src/libs/installer/extractarchiveoperation.cpp
@@ -193,20 +193,7 @@ bool ExtractArchiveOperation::performOperation()
 
     QFileInfo targetDirectoryInfo(fileDirectory);
 
-    // We need to create the directory structure step-by-step to avoid a rare race condition
-    // on Windows. QDir::mkpath() doesn't check if the leading directories were created elsewhere
-    // (like from within another thread) after the initial check that the given path requires
-    // creating also parent directories.
-    //
-    // On Unix patforms this case is handled by QFileSystemEngine though.
-    QDir resourcesDir(resourcesPath);
-    if (!resourcesDir.exists())
-        resourcesDir.mkdir(resourcesPath);
-
-    QDir resourceFileDir(targetDirectoryInfo.absolutePath());
-    if (!resourceFileDir.exists())
-        resourceFileDir.mkdir(targetDirectoryInfo.absolutePath());
-
+    QInstaller::createDirectoryWithParents(targetDirectoryInfo.absolutePath());
     setDefaultFilePermissions(resourcesPath, DefaultFilePermissions::Executable);
     setDefaultFilePermissions(targetDirectoryInfo.absolutePath(), DefaultFilePermissions::Executable);
 
diff --git a/src/libs/installer/fileutils.cpp b/src/libs/installer/fileutils.cpp
index 5a89073fc..11ae397fd 100644
--- a/src/libs/installer/fileutils.cpp
+++ b/src/libs/installer/fileutils.cpp
@@ -449,6 +449,52 @@ void QInstaller::mkpath(const QString &path)
 /*!
     \internal
 
+    Creates directory \a path including all parent directories. Return \c true on
+    success, \c false otherwise.
+
+    On Windows \c QDir::mkpath() doesn't check if the leading directories were created
+    elsewhere (i.e. in another thread) after the initial check that the given path
+    requires creating also parent directories, and returns \c false.
+
+    On Unix platforms this case is handled different by QFileSystemEngine though,
+    which checks for \c EEXIST error code in case any of the recursive directories
+    could not be created.
+
+    Compared to \c QInstaller::mkpath() and \c QDir::mkpath() this function checks if
+    each parent directory to-be-created were created elsewhere.
+*/
+bool QInstaller::createDirectoryWithParents(const QString &path)
+{
+    if (path.isEmpty())
+        return false;
+
+    QFileInfo dirInfo(path);
+    if (dirInfo.exists() && dirInfo.isDir())
+        return true;
+
+    // bail out if we are at the root directory
+    if (dirInfo.isRoot())
+        return false;
+
+    QDir dir(path);
+    if (dir.mkdir(path))
+        return true;
+
+    // mkdir failed, try to create the parent directory
+    if (!createDirectoryWithParents(dirInfo.absolutePath()))
+        return false;
+
+    // now try again
+    if (dir.mkdir(path))
+        return true;
+
+    // directory may be have also been created elsewhere
+    return (dirInfo.exists() && dirInfo.isDir());
+}
+
+/*!
+    \internal
+
     Generates and returns a temporary file name. The name can start with
     a template \a templ.
 */
diff --git a/src/libs/installer/fileutils.h b/src/libs/installer/fileutils.h
index 8b79ce052..1114cd60c 100644
--- a/src/libs/installer/fileutils.h
+++ b/src/libs/installer/fileutils.h
@@ -89,6 +89,7 @@ private:
 
     void INSTALLER_EXPORT mkdir(const QString &path);
     void INSTALLER_EXPORT mkpath(const QString &path);
+    bool INSTALLER_EXPORT createDirectoryWithParents(const QString &path);
 #ifdef Q_OS_MACOS
     void INSTALLER_EXPORT mkalias(const QString &path, const QString &alias);
 #endif