From: Christian Kamm Date: Tue, 5 Mar 2019 12:20:09 +0000 (+0100) Subject: FolderWatcher linux: Make automatically recursive #7068 X-Git-Tag: archive/raspbian/3.16.7-1_deb13u1+rpi1~1^2~12^2~22^2~47^2~4 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=74382ddcc0f809fcd187e259d26c6e76e7f141d2;p=nextcloud-desktop.git 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. --- diff --git a/src/gui/folder.cpp b/src/gui/folder.cpp index 2a1319d2c..aad3c0b6f 100644 --- a/src/gui/folder.cpp +++ b/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); diff --git a/src/gui/folderwatcher.cpp b/src/gui/folderwatcher.cpp index 959c9cacd..68aa57e9c 100644 --- a/src/gui/folderwatcher.cpp +++ b/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 diff --git a/src/gui/folderwatcher.h b/src/gui/folderwatcher.h index b07c33a10..0a30ef482 100644 --- a/src/gui/folderwatcher.h +++ b/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. */ diff --git a/src/gui/folderwatcher_linux.cpp b/src/gui/folderwatcher_linux.cpp index 7c207c4f5..2feda4e5e 100644 --- a/src/gui/folderwatcher_linux.cpp +++ b/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.")); - } - } - } -} - -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()); + if (path.isEmpty()) + return; + + 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::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; } } diff --git a/src/gui/folderwatcher_linux.h b/src/gui/folderwatcher_linux.h index 4501f8f3b..ba651f700 100644 --- a/src/gui/folderwatcher_linux.h +++ b/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 _watches; + QHash _watchToPath; + QMap _pathToWatch; QScopedPointer _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 _potentialRenames; - - QTimer *_wipePotentialRenamesSoon; }; } diff --git a/src/gui/folderwatcher_mac.h b/src/gui/folderwatcher_mac.h index 35da12c35..4ec666945 100644 --- a/src/gui/folderwatcher_mac.h +++ b/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 &); diff --git a/src/gui/folderwatcher_win.h b/src/gui/folderwatcher_win.h index 7ea673084..a556e9146 100644 --- a/src/gui/folderwatcher_win.h +++ b/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; diff --git a/test/testfolderwatcher.cpp b/test/testfolderwatcher.cpp index 9d44c85cd..7345e6450 100644 --- a/test/testfolderwatcher.cpp +++ b/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() {