QFileSystemModel/Win: Fix file system operations failing due to watchers

File system operations like renaming/removing may fail on Windows when
file system watchers are present. Add functions to
QFileSystemModelPrivate to temporarily remove the watchers prior to such
operations and to restore them in case of failure. Use them for
rename/remove (within a feature check for QFileSystemWatcher
and Q_OS_WIN).

Task-number: QTBUG-65683
Change-Id: I90142901892fbf9b1e1206a3397a95ffd3c8f010
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>

src/widgets/dialogs/qfileinfogatherer_p.h

@@ -167,6 +167,13 @@ public:
     explicit QFileInfoGatherer(QObject *parent = 0);
     ~QFileInfoGatherer();
 
+#if QT_CONFIG(filesystemwatcher) && defined(Q_OS_WIN)
+    QStringList watchedFiles() const            { return watcher->files(); }
+    QStringList watchedDirectories() const      { return watcher->directories(); }
+    void watchPaths(const QStringList &paths)   { watcher->addPaths(paths); }
+    void unwatchPaths(const QStringList &paths) { watcher->removePaths(paths); }
+#endif // filesystemwatcher && Q_OS_WIN
+
     // only callable from this->thread():
     void clear();
     void removePath(const QString &path);

src/widgets/dialogs/qfilesystemmodel.cpp

@@ -207,14 +207,17 @@ bool QFileSystemModel::remove(const QModelIndex &aindex)
 
     const QString path = d->filePath(aindex);
     const QFileInfo fileInfo(path);
+#if QT_CONFIG(filesystemwatcher) && defined(Q_OS_WIN)
+    // QTBUG-65683: Remove file system watchers prior to deletion to prevent
+    // failure due to locked files on Windows.
+    const QStringList watchedPaths = d->unwatchPathsAt(aindex);
+#endif // filesystemwatcher && Q_OS_WIN
     const bool success = (fileInfo.isFile() || fileInfo.isSymLink())
             ? QFile::remove(path) : QDir(path).removeRecursively();
-#ifndef QT_NO_FILESYSTEMWATCHER
-    if (success) {
-        QFileSystemModelPrivate * d = const_cast<QFileSystemModelPrivate*>(d_func());
-        d->fileInfoGatherer.removePath(path);
-    }
-#endif
+#if QT_CONFIG(filesystemwatcher) && defined(Q_OS_WIN)
+    if (!success)
+        d->watchPaths(watchedPaths);
+#endif // filesystemwatcher && Q_OS_WIN
     return success;
 }
 
@@ -851,6 +854,20 @@ QIcon QFileSystemModelPrivate::icon(const QModelIndex &index) const
     return node(index)->icon();
 }
 
+static void displayRenameFailedMessage(const QString &newName)
+{
+#if QT_CONFIG(messagebox)
+    const QString message =
+        QFileSystemModel::tr("<b>The name \"%1\" cannot be used.</b>"
+                             "<p>Try using another name, with fewer characters or no punctuation marks.")
+                             .arg(newName);
+    QMessageBox::information(nullptr, QFileSystemModel::tr("Invalid filename"),
+                             message, QMessageBox::Ok);
+#else
+    Q_UNUSED(newName)
+#endif // QT_CONFIG(messagebox)
+}
+
 /*!
     \reimp
 */
@@ -871,15 +888,21 @@ bool QFileSystemModel::setData(const QModelIndex &idx, const QVariant &value, in
 
     const QString parentPath = filePath(parent(idx));
 
-    if (newName.isEmpty()
-        || QDir::toNativeSeparators(newName).contains(QDir::separator())
-        || !QDir(parentPath).rename(oldName, newName)) {
-#if QT_CONFIG(messagebox)
-        QMessageBox::information(0, QFileSystemModel::tr("Invalid filename"),
-                                QFileSystemModel::tr("<b>The name \"%1\" can not be used.</b><p>Try using another name, with fewer characters or no punctuations marks.")
-                                .arg(newName),
-                                 QMessageBox::Ok);
-#endif // QT_CONFIG(messagebox)
+    if (newName.isEmpty() || QDir::toNativeSeparators(newName).contains(QDir::separator())) {
+        displayRenameFailedMessage(newName);
+        return false;
+    }
+
+#if QT_CONFIG(filesystemwatcher) && defined(Q_OS_WIN)
+    // QTBUG-65683: Remove file system watchers prior to renaming to prevent
+    // failure due to locked files on Windows.
+    const QStringList watchedPaths = d->unwatchPathsAt(idx);
+#endif // filesystemwatcher && Q_OS_WIN
+    if (!QDir(parentPath).rename(oldName, newName)) {
+#if QT_CONFIG(filesystemwatcher) && defined(Q_OS_WIN)
+        d->watchPaths(watchedPaths);
+#endif
+        displayRenameFailedMessage(newName);
         return false;
     } else {
         /*
@@ -1882,6 +1905,46 @@ void QFileSystemModelPrivate::_q_resolvedName(const QString &fileName, const QSt
     resolvedSymLinks[fileName] = resolvedName;
 }
 
+#if QT_CONFIG(filesystemwatcher) && defined(Q_OS_WIN)
+// Remove file system watchers at/below the index and return a list of previously
+// watched files. This should be called prior to operations like rename/remove
+// which might fail due to watchers on platforms like Windows. The watchers
+// should be restored on failure.
+QStringList QFileSystemModelPrivate::unwatchPathsAt(const QModelIndex &index)
+{
+    const QFileSystemModelPrivate::QFileSystemNode *indexNode = node(index);
+    if (indexNode == nullptr)
+        return QStringList();
+    const Qt::CaseSensitivity caseSensitivity = indexNode->caseSensitive()
+        ? Qt::CaseSensitive : Qt::CaseInsensitive;
+    const QString path = indexNode->fileInfo().absoluteFilePath();
+
+    QStringList result;
+    const auto filter = [path, caseSensitivity] (const QString &watchedPath)
+    {
+        const int pathSize = path.size();
+        if (pathSize == watchedPath.size()) {
+            return path.compare(watchedPath, caseSensitivity) == 0;
+        } else if (watchedPath.size() > pathSize) {
+            return watchedPath.at(pathSize) == QLatin1Char('/')
+                && watchedPath.startsWith(path, caseSensitivity);
+        }
+        return false;
+    };
+
+    const QStringList &watchedFiles = fileInfoGatherer.watchedFiles();
+    std::copy_if(watchedFiles.cbegin(), watchedFiles.cend(),
+                 std::back_inserter(result), filter);
+
+    const QStringList &watchedDirectories = fileInfoGatherer.watchedDirectories();
+    std::copy_if(watchedDirectories.cbegin(), watchedDirectories.cend(),
+                 std::back_inserter(result), filter);
+
+    fileInfoGatherer.unwatchPaths(result);
+    return result;
+}
+#endif // filesystemwatcher && Q_OS_WIN
+
 /*!
     \internal
 */

src/widgets/dialogs/qfilesystemmodel_p.h

@@ -297,9 +297,13 @@ public:
     static int naturalCompare(const QString &s1, const QString &s2, Qt::CaseSensitivity cs);
 
     QDir rootDir;
-#ifndef QT_NO_FILESYSTEMWATCHER
+#if QT_CONFIG(filesystemwatcher)
+#  ifdef Q_OS_WIN
+    QStringList unwatchPathsAt(const QModelIndex &);
+    void watchPaths(const QStringList &paths) { fileInfoGatherer.watchPaths(paths); }
+#  endif // Q_OS_WIN
     QFileInfoGatherer fileInfoGatherer;
-#endif
+#endif // filesystemwatcher
     QTimer delayedSortTimer;
     bool forceSort;
     int sortColumn;

tests/auto/widgets/dialogs/qfilesystemmodel/tst_qfilesystemmodel.cpp

@@ -102,6 +102,7 @@ private slots:
 
     void mkdir();
     void deleteFile();
+    void deleteDirectory();
 
     void caseSensitivity();
 
@@ -884,6 +885,44 @@ void tst_QFileSystemModel::deleteFile()
     QVERIFY(!newFile.exists());
 }
 
+void tst_QFileSystemModel::deleteDirectory()
+{
+    // QTBUG-65683: Verify that directories can be removed recursively despite
+    // file system watchers being active on them or their sub-directories (Windows).
+    // Create a temporary directory, a nested directory and expand a treeview
+    // to show them to ensure watcher creation. Then delete the directory.
+    QTemporaryDir dirToBeDeleted(flatDirTestPath + QStringLiteral("/deleteDirectory-XXXXXX"));
+    QVERIFY(dirToBeDeleted.isValid());
+    const QString dirToBeDeletedPath = dirToBeDeleted.path();
+    const QString nestedTestDir = QStringLiteral("test");
+    QVERIFY(QDir(dirToBeDeletedPath).mkpath(nestedTestDir));
+    const QString nestedTestDirPath = dirToBeDeletedPath + QLatin1Char('/') + nestedTestDir;
+    QFile testFile(nestedTestDirPath + QStringLiteral("/test.txt"));
+    QVERIFY(testFile.open(QIODevice::WriteOnly | QIODevice::Text));
+    testFile.write("Hello\n");
+    testFile.close();
+
+    QFileSystemModel model;
+    const QModelIndex rootIndex = model.setRootPath(flatDirTestPath);
+    QTreeView treeView;
+    treeView.setWindowTitle(QTest::currentTestFunction());
+    treeView.setModel(&model);
+    treeView.setRootIndex(rootIndex);
+
+    const QModelIndex dirToBeDeletedPathIndex = model.index(dirToBeDeletedPath);
+    QVERIFY(dirToBeDeletedPathIndex.isValid());
+    treeView.setExpanded(dirToBeDeletedPathIndex, true);
+    const QModelIndex nestedTestDirIndex = model.index(nestedTestDirPath);
+    QVERIFY(nestedTestDirIndex.isValid());
+    treeView.setExpanded(nestedTestDirIndex, true);
+
+    treeView.show();
+    QVERIFY(QTest::qWaitForWindowExposed(&treeView));
+
+    QVERIFY(model.remove(dirToBeDeletedPathIndex));
+    dirToBeDeleted.setAutoRemove(false);
+}
+
 static QString flipCase(QString s)
 {
     for (int i = 0, size = s.size(); i < size; ++i) {