From 104200d688650db02f1447f73a499784115d897d Mon Sep 17 00:00:00 2001 From: Filippo Cucchetto Date: Sun, 21 Oct 2018 10:36:17 +0200 Subject: [PATCH] Fix QFileSystemWatcher::removePath after move operations Foreword: - During a file or directory move the inotify id for an entity is not changed. - QFileSystemWatcher implementation uses a QMultiHash for mapping an id to its path. Suppose this filesystem hypothetical directory structure - A |--> B and user watches both A and B directories. Suppose that the B directory gets moved by calling "mv B B1". The user receives a directoryChanged event for parent directory A and scan filesystem for changes. During this scan the user notices: - a new directory B1 - a deleted directory B The user simply invoke QFileSystemWatcher::addPath(B1) and QFileSystemWatcher::removePath(B). With the actual implementation the second operation could fail: - The call QFileSystemWatcher::addPath(B1) insert a duplicated records in the QFileSystemWatcher::idToPath multihash ( {0, "A"}, {1, "A/B"} {1, "A/B1"} - The call QFileSystemWatcher::removePath(B) fails because - it first retrieves the the id for path B ---> pathToId("A/B") <-- return 1 - Then it calls idToPath.take with the id obtain in the previous step <--- idToPath.take(1) This last operation could take the record {1, "A/B1"} instead of the {1, "A/B"} (because both have the same key) meaning that the next check "x != path" evaluates to true (making the removePath function fail). This patch fixes the problem in the following way: - Use idToPath.equal_range in order to obtain all paths with the given id - Remove the correct record that match (id, path) - Prevent the removal of the inotify watch if another record with the same id is still present in the multihash (like a simple reference counting). Change-Id: I9c8480b2a869d91e500af5c4aded596b9aa53b46 Reviewed-by: Edward Welbourne --- src/corelib/io/qfilesystemwatcher_inotify.cpp | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/corelib/io/qfilesystemwatcher_inotify.cpp b/src/corelib/io/qfilesystemwatcher_inotify.cpp index 3b7135e582b..a5e629b6467 100644 --- a/src/corelib/io/qfilesystemwatcher_inotify.cpp +++ b/src/corelib/io/qfilesystemwatcher_inotify.cpp @@ -330,13 +330,24 @@ QStringList QInotifyFileSystemWatcherEngine::removePaths(const QStringList &path while (it.hasNext()) { QString path = it.next(); int id = pathToID.take(path); - QString x = idToPath.take(id); - if (x.isEmpty() || x != path) + + // Multiple paths could be associated to the same watch descriptor + // when a file is moved and added with the new name. + // So we should find and delete the correct one by using + // both id and path + auto path_range = idToPath.equal_range(id); + auto path_it = std::find(path_range.first, path_range.second, path); + if (path_it == idToPath.end()) continue; - int wd = id < 0 ? -id : id; - // qDebug() << "removing watch for path" << path << "wd" << wd; - inotify_rm_watch(inotifyFd, wd); + const ssize_t num_elements = std::distance(path_range.first, path_range.second); + idToPath.erase(path_it); + + // If there was only one path associated to the given id we should remove the watch + if (num_elements == 1) { + int wd = id < 0 ? -id : id; + inotify_rm_watch(inotifyFd, wd); + } it.remove(); if (id < 0) {