FolderWatcher linux: Make automatically recursive #7068

Previously it depended on addFolder() / removeFolder() calls to adjust
watchers when new folders were added or removed. There also needed to be
complex move handling.

Now, any folder creation/move-in notifications automatically trigger
watcher additions and folder deletion/move-out triggers removal.

src/gui/folder.cpp

@@ -898,43 +898,6 @@ void Folder::slotItemCompleted(const SyncFileItemPtr &item)
         return;
     }
 
-    // add new directories or remove gone away dirs to the watcher
-    if (_folderWatcher && item->isDirectory()) {
-    switch (item->_instruction) {
-        case CSYNC_INSTRUCTION_NEW:
-                _folderWatcher->addPath(path() + item->_file);
-        break;
-        case CSYNC_INSTRUCTION_REMOVE:
-                _folderWatcher->removePath(path() + item->_file);
-        break;
-        case CSYNC_INSTRUCTION_RENAME:
-                _folderWatcher->removePath(path() + item->_file);
-                _folderWatcher->addPath(path() + item->destination());
-        break;
-        default:
-        break;
-    }
-    }
-
-    // Success and failure of sync items adjust what the next sync is
-    // supposed to do.
-    //
-    // For successes, we want to wipe the file from the list to ensure we don't
-    // rediscover it even if this overall sync fails.
-    //
-    // For failures, we want to add the file to the list so the next sync
-    // will be able to retry it.
-    if (item->_status == SyncFileItem::Success
-        || item->_status == SyncFileItem::FileIgnored
-        || item->_status == SyncFileItem::Restoration
-        || item->_status == SyncFileItem::Conflict) {
-        if (_previousLocalDiscoveryPaths.erase(item->_file.toUtf8()))
-            qCDebug(lcFolder) << "local discovery: wiped" << item->_file;
-    } else {
-        _localDiscoveryPaths.insert(item->_file.toUtf8());
-        qCDebug(lcFolder) << "local discovery: inserted" << item->_file << "due to sync failure";
-    }
-
     _syncResult.processCompletedItem(item);
 
     _fileLog->logItem(*item);

src/gui/folderwatcher.cpp

@@ -86,6 +86,15 @@ void FolderWatcher::appendSubPaths(QDir dir, QStringList& subPaths) {
     }
 }
 
+int FolderWatcher::testLinuxWatchCount() const
+{
+#ifdef Q_OS_LINUX
+    return _d->testWatchCount();
+#else
+    return -1;
+#endif
+}
+
 void FolderWatcher::changeDetected(const QString &path)
 {
     QFileInfo fileInfo(path);
@@ -134,15 +143,4 @@ void FolderWatcher::changeDetected(const QStringList &paths)
     }
 }
 
-void FolderWatcher::addPath(const QString &path)
-{
-    _d->addPath(path);
-}
-
-void FolderWatcher::removePath(const QString &path)
-{
-    _d->removePath(path);
-}
-
-
 } // namespace OCC

src/gui/folderwatcher.h

@@ -44,11 +44,6 @@ class Folder;
  * for changes in the local file system. Changes are signalled
  * through the pathChanged() signal.
  *
- * Note that if new folders are created, this folderwatcher class
- * does not automatically add them to the list of monitored
- * dirs. That is the responsibility of the user of this class to
- * call addPath() with the new dir.
- *
  * @ingroup gui
  */
 
@@ -65,14 +60,6 @@ public:
      */
     void init(const QString &root);
 
-    /**
-     * Not all backends are recursive by default.
-     * Those need to be notified when a directory is added or removed while the watcher is disabled.
-     * This is a no-op for backends that are recursive
-     */
-    void addPath(const QString &);
-    void removePath(const QString &);
-
     /* Check if the path is ignored. */
     bool pathIsIgnored(const QString &path);
 
@@ -85,6 +72,9 @@ public:
      */
     bool isReliable() const;
 
+    /// For testing linux behavior only
+    int testLinuxWatchCount() const;
+
 signals:
     /** Emitted when one of the watched directories or one
      *  of the contained files is changed. */

src/gui/folderwatcher_linux.cpp

@@ -31,11 +31,6 @@ FolderWatcherPrivate::FolderWatcherPrivate(FolderWatcher *p, const QString &path
     , _parent(p)
     , _folder(path)
 {
-    _wipePotentialRenamesSoon = new QTimer(this);
-    _wipePotentialRenamesSoon->setInterval(1000);
-    _wipePotentialRenamesSoon->setSingleShot(true);
-    connect(_wipePotentialRenamesSoon, &QTimer::timeout, this, &FolderWatcherPrivate::wipePotentialRenames);
-
     _fd = inotify_init();
     if (_fd != -1) {
         _socket.reset(new QSocketNotifier(_fd, QSocketNotifier::Read));
@@ -76,45 +71,37 @@ bool FolderWatcherPrivate::findFoldersBelow(const QDir &dir, QStringList &fullLi
 
 void FolderWatcherPrivate::inotifyRegisterPath(const QString &path)
 {
-    if (!path.isEmpty()) {
-        int wd = inotify_add_watch(_fd, path.toUtf8().constData(),
-            IN_CLOSE_WRITE | IN_ATTRIB | IN_MOVE | IN_CREATE | IN_DELETE | IN_DELETE_SELF | IN_MOVE_SELF | IN_UNMOUNT | IN_ONLYDIR);
-        if (wd > -1) {
-            _watches.insert(wd, path);
-        } else {
-            // If we're running out of memory or inotify watches, become
-            // unreliable.
-            if (_parent->_isReliable && (errno == ENOMEM || errno == ENOSPC)) {
-                _parent->_isReliable = false;
-                emit _parent->becameUnreliable(
-                    tr("This problem usually happens when the inotify watches are exhausted. "
-                       "Check the FAQ for details."));
-            }
-        }
-    }
-}
+    if (path.isEmpty())
+        return;
 
-void FolderWatcherPrivate::applyDirectoryRename(const FolderWatcherPrivate::Rename &rename)
-{
-    QString fromSlash = rename.from + "/";
-    qCInfo(lcFolderWatcher) << "Applying rename from" << rename.from << "to" << rename.to;
-    for (auto &watch : _watches) {
-        if (watch == rename.from || watch.startsWith(fromSlash)) {
-            watch = rename.to + watch.mid(rename.from.size());
+    int wd = inotify_add_watch(_fd, path.toUtf8().constData(),
+        IN_CLOSE_WRITE | IN_ATTRIB | IN_MOVE | IN_CREATE | IN_DELETE | IN_DELETE_SELF | IN_MOVE_SELF | IN_UNMOUNT | IN_ONLYDIR);
+    if (wd > -1) {
+        _watchToPath.insert(wd, path);
+        _pathToWatch.insert(path, wd);
+    } else {
+        // If we're running out of memory or inotify watches, become
+        // unreliable.
+        if (_parent->_isReliable && (errno == ENOMEM || errno == ENOSPC)) {
+            _parent->_isReliable = false;
+            emit _parent->becameUnreliable(
+                tr("This problem usually happens when the inotify watches are exhausted. "
+                   "Check the FAQ for details."));
         }
     }
 }
 
 void FolderWatcherPrivate::slotAddFolderRecursive(const QString &path)
 {
+    if (_pathToWatch.contains(path))
+        return;
+
     int subdirs = 0;
     qCDebug(lcFolderWatcher) << "(+) Watcher:" << path;
 
     QDir inPath(path);
     inotifyRegisterPath(inPath.absolutePath());
 
-    const QStringList watchedFolders = _watches.values();
-
     QStringList allSubfolders;
     if (!findFoldersBelow(QDir(path), allSubfolders)) {
         qCWarning(lcFolderWatcher) << "Could not traverse all sub folders";
@@ -123,7 +110,7 @@ void FolderWatcherPrivate::slotAddFolderRecursive(const QString &path)
     while (subfoldersIt.hasNext()) {
         QString subfolder = subfoldersIt.next();
         QDir folder(subfolder);
-        if (folder.exists() && !watchedFolders.contains(folder.absolutePath())) {
+        if (folder.exists() && !_pathToWatch.contains(folder.absolutePath())) {
             subdirs++;
             if (_parent->pathIsIgnored(subfolder)) {
                 qCDebug(lcFolderWatcher) << "* Not adding" << folder.path();
@@ -140,11 +127,6 @@ void FolderWatcherPrivate::slotAddFolderRecursive(const QString &path)
     }
 }
 
-void FolderWatcherPrivate::wipePotentialRenames()
-{
-    _potentialRenames.clear();
-}
-
 void FolderWatcherPrivate::slotReceivedNotification(int fd)
 {
     int len = 0;
@@ -193,48 +175,44 @@ void FolderWatcherPrivate::slotReceivedNotification(int fd)
             || fileName.startsWith(".sync_")) {
             continue;
         }
-        const QString p = _watches[event->wd] + '/' + fileName;
+        const QString p = _watchToPath[event->wd] + '/' + fileName;
         _parent->changeDetected(p);
 
-        // Collect events to form complete renames where possible
-        // and apply directory renames to the cached paths.
-        if ((event->mask & (IN_MOVED_TO | IN_MOVED_FROM)) && (event->mask & IN_ISDIR) && event->cookie > 0) {
-            auto &rename = _potentialRenames[event->cookie];
-            if (event->mask & IN_MOVED_TO)
-                rename.to = p;
-            if (event->mask & IN_MOVED_FROM)
-                rename.from = p;
-            if (!rename.from.isEmpty() && !rename.to.isEmpty()) {
-                applyDirectoryRename(rename);
-                _potentialRenames.remove(event->cookie);
-            } else {
-                _wipePotentialRenamesSoon->start();
-            }
+        if ((event->mask & (IN_MOVED_TO | IN_CREATE))
+            && QFileInfo(p).isDir()
+            && !_parent->pathIsIgnored(p)) {
+            slotAddFolderRecursive(p);
+        }
+        if (event->mask & (IN_MOVED_FROM | IN_DELETE)) {
+            removeFoldersBelow(p);
         }
     }
 }
 
-void FolderWatcherPrivate::addPath(const QString &path)
+void FolderWatcherPrivate::removeFoldersBelow(const QString &path)
 {
-    slotAddFolderRecursive(path);
-}
+    auto it = _pathToWatch.find(path);
+    if (it == _pathToWatch.end())
+        return;
 
-void FolderWatcherPrivate::removePath(const QString &path)
-{
-    int wid = -1;
-    // Remove the inotify watch.
-    QHash<int, QString>::const_iterator i = _watches.constBegin();
+    QString pathSlash = path + '/';
 
-    while (i != _watches.constEnd()) {
-        if (i.value() == path) {
-            wid = i.key();
+    // Remove the entry and all subentries
+    while (it != _pathToWatch.end()) {
+        auto itPath = it.key();
+        if (!itPath.startsWith(path))
             break;
+        if (itPath != path && !itPath.startsWith(pathSlash)) {
+            // order is 'foo', 'foo bar', 'foo/bar'
+            ++it;
+            continue;
         }
-        ++i;
-    }
-    if (wid > -1) {
+
+        auto wid = it.value();
         inotify_rm_watch(_fd, wid);
-        _watches.remove(wid);
+        _watchToPath.remove(wid);
+        it = _pathToWatch.erase(it);
+        qCDebug(lcFolderWatcher) << "Removed watch for" << itPath;
     }
 }
 

src/gui/folderwatcher_linux.h

@@ -39,55 +39,25 @@ public:
     FolderWatcherPrivate(FolderWatcher *p, const QString &path);
     ~FolderWatcherPrivate();
 
-    void addPath(const QString &path);
-    void removePath(const QString &);
+    int testWatchCount() const { return _pathToWatch.size(); }
 
 protected slots:
     void slotReceivedNotification(int fd);
     void slotAddFolderRecursive(const QString &path);
 
-    /// Remove all half-built renames. Called by timer when idle for a bit.
-    void wipePotentialRenames();
-
 protected:
-    struct Rename
-    {
-        QString from;
-        QString to;
-    };
-
     bool findFoldersBelow(const QDir &dir, QStringList &fullList);
     void inotifyRegisterPath(const QString &path);
-
-    /// Adjusts the paths in _watches when directories are renamed.
-    void applyDirectoryRename(const Rename &rename);
+    void removeFoldersBelow(const QString &path);
 
 private:
     FolderWatcher *_parent;
 
     QString _folder;
-    QHash<int, QString> _watches;
+    QHash<int, QString> _watchToPath;
+    QMap<QString, int> _pathToWatch;
     QScopedPointer<QSocketNotifier> _socket;
     int _fd;
-
-    /** Maps inotify event cookie to rename data.
-     *
-     * For moves two independent inotify events will be seen and they
-     * can be matched via the event cookie. This field stores partial
-     * information as it is received. When both sides have arrived,
-     * directory moves can be processed with applyDirectoryRename().
-     *
-     * If we don't receive both sides (if something moves away from
-     * the watched folder tree, or into it from an unwatched location)
-     * the _wipePotentialRenamesSoon will eventually discard the
-     * incomplete data.
-     *
-     * These events can even be emitted by different watches if the
-     * directory parent folder changed.
-     */
-    QHash<quint32, Rename> _potentialRenames;
-
-    QTimer *_wipePotentialRenamesSoon;
 };
 }
 

src/gui/folderwatcher_mac.h

@@ -33,9 +33,6 @@ public:
     FolderWatcherPrivate(FolderWatcher *p, const QString &path);
     ~FolderWatcherPrivate();
 
-    void addPath(const QString &) {}
-    void removePath(const QString &) {}
-
     void startWatching();
     QStringList addCoalescedPaths(const QStringList &) const;
     void doNotifyParent(const QStringList &);

src/gui/folderwatcher_win.h

@@ -74,9 +74,6 @@ public:
     FolderWatcherPrivate(FolderWatcher *p, const QString &path);
     ~FolderWatcherPrivate();
 
-    void addPath(const QString &) {}
-    void removePath(const QString &) {}
-
 private:
     FolderWatcher *_parent;
     WatcherThread *_thread;

test/testfolderwatcher.cpp

@@ -96,6 +96,12 @@ class TestFolderWatcher : public QObject
         return false;
     }
 
+#ifdef Q_OS_LINUX
+#define CHECK_WATCH_COUNT(n) QCOMPARE(_watcher->testLinuxWatchCount(), (n))
+#else
+#define CHECK_WATCH_COUNT(n) do {} while (false)
+#endif
+
 public:
     TestFolderWatcher() {
         qsrand(QTime::currentTime().msec());
@@ -118,10 +124,24 @@ public:
         _pathChangedSpy.reset(new QSignalSpy(_watcher.data(), SIGNAL(pathChanged(QString))));
     }
 
+    int countFolders(const QString &path)
+    {
+        int n = 0;
+        for (const auto &sub : QDir(path).entryList(QDir::Dirs | QDir::NoDotAndDotDot))
+            n += 1 + countFolders(path + '/' + sub);
+        return n;
+    }
+
 private slots:
     void init()
     {
         _pathChangedSpy->clear();
+        CHECK_WATCH_COUNT(countFolders(_rootPath) + 1);
+    }
+
+    void cleanup()
+    {
+        CHECK_WATCH_COUNT(countFolders(_rootPath) + 1);
     }
 
     void testACreate() { // create a new file
@@ -155,6 +175,11 @@ private slots:
         QString file(_rootPath+"/a1/b1/new_dir");
         mkdir(file);
         QVERIFY(waitForPathChanged(file));
+
+        // Notifications from that new folder arrive too
+        QString file2(_rootPath + "/a1/b1/new_dir/contained");
+        touch(file2);
+        QVERIFY(waitForPathChanged(file2));
     }
 
     void testRemoveADir() {