fix(macOS): handle file system events off the GUI thread

The FSEvents stream was scheduled on the main dispatch queue, so every event
batch was handled on the GUI thread. Dropping a large tree into the sync folder
made that work unbounded: addCoalescedPaths() walked each reported directory's
whole subtree with QDirIterator and deduplicated it with a linear
QStringList::contains(), and on kFSEventStreamEventFlagMustScanSubDirs or a
kernel drop -- exactly what a mass copy triggers -- notifyAll() enumerated the
entire sync folder and reported every path one by one. Each reported path then
costs a journal lookup and several stats in Folder::slotWatchedPathChanged(),
so the window stayed frozen until the whole batch was processed.

Deliver events on a private serial queue instead, as the Windows backend
already does with its own WatcherThread, and hand each batch over to the
FolderWatcher's thread with a queued invocation. Deduplicate the expansion with
a QSet rather than a linear scan, and stop expanding past maxCoalescedPaths:
nothing is lost by stopping, because the directory being expanded was itself
reported and SyncEngine::shouldDiscoverLocally() descends into a touched
directory's entire subtree.

notifyAll() no longer walks the tree either: a full local discovery re-reads it
anyway, so it just emits lostChanges(). It reports no path deliberately -- the
watched root would reach the local discovery tracker as an empty relative path,
which matches only the root itself and not the tree below it.

Signed-off-by: Felipe Tumonis <ftumonis@gmail.com>

Author: ftapps

src/gui/folderwatcher.cpp

@@ -213,6 +213,17 @@ int FolderWatcher::lockChangeDebouncingTimout() const
     return _lockChangeDebouncingTimer.interval();
 }
 
+void FolderWatcher::changesLost(const QString &root)
+{
+    qCWarning(lcFolderWatcher) << "Lost track of individual changes in" << root
+                               << "- the next sync will run a full local discovery";
+
+    // Makes the next sync rediscover the tree from the filesystem instead of the database. No path
+    // is reported: the root would reach the local discovery tracker as an empty relative path,
+    // which matches only the root itself and not the tree below it.
+    emit lostChanges();
+}
+
 void FolderWatcher::changeDetected(const QString &path)
 {
     QStringList paths(path);

src/gui/folderwatcher.h

@@ -129,6 +129,14 @@ private slots:
     void startNotificationTestWhenReady();
     void lockChangeDebouncingTimerTimedOut();
 
+private:
+    /** Report that the backend could not keep track of individual changes under @a root.
+     *
+     * Emits lostChanges() so that the next sync runs a full local discovery. Reports no path, so
+     * whoever wants a sync scheduled has to react to lostChanges() as well.
+     */
+    void changesLost(const QString &root);
+
 protected:
     QHash<QString, int> _pendingPathes;
 

src/gui/folderwatcher_mac.cpp

@@ -13,11 +13,22 @@
 
 #include <cerrno>
 #include <QDirIterator>
+#include <QSet>
 #include <QStringList>
 
 
 namespace OCC {
 
+namespace {
+/* Upper bound on the paths one event batch may be expanded into by addCoalescedPaths().
+ *
+ * Every reported path costs a journal lookup and a few stats on the GUI thread later on, so for a
+ * large tree it is both faster and gentler on the UI to stop enumerating and let the next sync
+ * rediscover the tree instead.
+ */
+constexpr auto maxCoalescedPaths = 1000;
+}
+
 FolderWatcherPrivate::FolderWatcherPrivate(FolderWatcher *p, const QString &path)
     : _parent(p)
     , _folder(path)
@@ -27,9 +38,20 @@ FolderWatcherPrivate::FolderWatcherPrivate(FolderWatcher *p, const QString &path
 
 FolderWatcherPrivate::~FolderWatcherPrivate()
 {
-    FSEventStreamStop(_stream);
-    FSEventStreamInvalidate(_stream);
-    FSEventStreamRelease(_stream);
+    if (_stream) {
+        FSEventStreamStop(_stream);
+        FSEventStreamInvalidate(_stream);
+        FSEventStreamRelease(_stream);
+        _stream = nullptr;
+    }
+
+    if (_queue) {
+        // A callback block may already be running on the queue and still touching this object,
+        // so let it drain before the members it reads are destroyed.
+        dispatch_sync(_queue, ^{});
+        dispatch_release(_queue);
+        _queue = nullptr;
+    }
 }
 
 static void callback(
@@ -108,44 +130,71 @@ void FolderWatcherPrivate::startWatching()
 
     CFRelease(pathsToWatch);
     CFRelease(folderCF);
-    FSEventStreamSetDispatchQueue(_stream, dispatch_get_main_queue());
+
+    // Deliver events on a private queue rather than the main one: dropping a large tree into the
+    // sync folder produces event batches big enough that handling them on the main queue blocks
+    // the GUI. This mirrors what the Windows watcher does with its own WatcherThread.
+    _queue = dispatch_queue_create("com.nextcloud.desktopclient.folderwatcher", DISPATCH_QUEUE_SERIAL);
+    FSEventStreamSetDispatchQueue(_stream, _queue);
     FSEventStreamStart(_stream);
 }
 
-QStringList FolderWatcherPrivate::addCoalescedPaths(const QStringList &paths) const
+void FolderWatcherPrivate::addCoalescedPaths(const QStringList &paths, QStringList &coalesced) const
 {
-    QStringList coalescedPaths;
+    // Only the expansion below is bounded, not `paths` itself: those are events FSEvents actually
+    // reported, they are capped by the kernel's own buffer, and reporting them is what drives
+    // things a rediscovery cannot redo, such as releasing office file locks.
+    //
+    // FSEvents reports a renamed or moved directory, but not the items below it, so the tree is
+    // walked to report those too.
+    coalesced = paths;
+    QSet<QString> seen{paths.begin(), paths.end()};
+
     for (const auto &eventPath : paths) {
-        if (QDir(eventPath).exists()) {
-            QDirIterator it(eventPath, QDir::AllDirs | QDir::NoDotAndDotDot | QDir::Files, QDirIterator::Subdirectories);
-            while (it.hasNext()) {
-                const auto path = it.next();
-                if (!paths.contains(path)) {
-                    coalescedPaths.append(path);
-                }
+        if (!QDir(eventPath).exists()) {
+            continue;
+        }
+        QDirIterator it(eventPath, QDir::AllDirs | QDir::NoDotAndDotDot | QDir::Files, QDirIterator::Subdirectories);
+        while (it.hasNext()) {
+            const auto path = it.next();
+            if (coalesced.size() >= maxCoalescedPaths) {
+                // Give up on expanding this batch rather than report a whole tree path by path.
+                // Nothing is lost by stopping: the directory that is being expanded was itself
+                // reported, and SyncEngine::shouldDiscoverLocally() descends into a touched
+                // directory's entire subtree, so the entries below it are discovered anyway.
+                return;
+            }
+            if (!seen.contains(path)) {
+                seen.insert(path);
+                coalesced.append(path);
             }
         }
     }
-    return (paths + coalescedPaths);
 }
 
 void FolderWatcherPrivate::doNotifyParent(const QStringList &paths)
 {
-    const QStringList totalPaths = addCoalescedPaths(paths);
-    _parent->changeDetected(totalPaths);
+    if (paths.isEmpty()) {
+        return;
+    }
+
+    QStringList totalPaths;
+    addCoalescedPaths(paths, totalPaths);
+
+    // We are on the private dispatch queue here, but everything FolderWatcher touches from
+    // changeDetected() (its timers, the lock-file sets, the pathChanged() receivers) may only be
+    // used on the thread it lives on, so hand the batch over instead of processing it here.
+    QMetaObject::invokeMethod(
+        _parent, [parent = _parent, totalPaths] { parent->changeDetected(totalPaths); }, Qt::QueuedConnection);
 }
 
 void FolderWatcherPrivate::notifyAll()
 {
-    QDirIterator dirIterator(_folder, QDirIterator::Subdirectories);
-    QStringList allPaths;
-
-    while(dirIterator.hasNext()) {
-        const auto dirEntry = dirIterator.next();
-        allPaths.append(dirEntry);
-    }
-
-    _parent->changeDetected(allPaths);
+    // FSEvents either dropped events or asked for a full rescan. Enumerating the whole tree to
+    // report every path would take minutes on a large folder, and a full local discovery re-reads
+    // it anyway, so ask for one of those instead of reporting anything.
+    QMetaObject::invokeMethod(
+        _parent, [parent = _parent, folder = _folder] { parent->changesLost(folder); }, Qt::QueuedConnection);
 }
 
 

src/gui/folderwatcher_mac.h

@@ -26,7 +26,14 @@ class FolderWatcherPrivate
     ~FolderWatcherPrivate();
 
     void startWatching();
-    QStringList addCoalescedPaths(const QStringList &) const;
+
+    /** Expand @a paths with the contents of any directory among them, into @a coalesced.
+     *
+     * The expansion stops at maxCoalescedPaths; the sync engine descends into a reported directory
+     * on its own, so the entries left out are still discovered. @a paths itself is always passed
+     * through, however large it is.
+     */
+    void addCoalescedPaths(const QStringList &paths, QStringList &coalesced) const;
     void doNotifyParent(const QStringList &);
     void notifyAll();
 
@@ -38,7 +45,10 @@ class FolderWatcherPrivate
 
     QString _folder;
 
-    FSEventStreamRef _stream;
+    FSEventStreamRef _stream = nullptr;
+
+    /// Events are delivered here instead of on the main queue, to keep them off the GUI thread.
+    dispatch_queue_t _queue = nullptr;
 };
 }