From 93a466c6fc3c455dcc9bf1292cc6a2725287a94a Mon Sep 17 00:00:00 2001 From: "Bradley T. Hughes" Date: Wed, 11 Jan 2012 09:00:41 +0100 Subject: [PATCH] Remove unnecessary QMutexes in QFileSystemWatcher implementations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The polling, inotify, and kqueue implementations are no longer threaded, and as such, do not need mutexes to protect their internal data (since QFileSystemWatcher itself is not documented as a thread-safe API). The Windows implementation is unchanged as it uses multiple threads explicitly. Change-Id: Ia82510397e576bf704ce3aed3d776b58b39f7ff3 Reviewed-by: Robin Burchell Reviewed-by: João Abecasis --- src/corelib/io/qfilesystemwatcher.cpp | 1 - src/corelib/io/qfilesystemwatcher_inotify.cpp | 6 - src/corelib/io/qfilesystemwatcher_inotify_p.h | 1 - src/corelib/io/qfilesystemwatcher_kqueue.cpp | 164 +++++++++--------- src/corelib/io/qfilesystemwatcher_kqueue_p.h | 1 - src/corelib/io/qfilesystemwatcher_polling.cpp | 3 - src/corelib/io/qfilesystemwatcher_polling_p.h | 1 - 7 files changed, 78 insertions(+), 99 deletions(-) diff --git a/src/corelib/io/qfilesystemwatcher.cpp b/src/corelib/io/qfilesystemwatcher.cpp index 85b27c9ec9..b6eb5eda5a 100644 --- a/src/corelib/io/qfilesystemwatcher.cpp +++ b/src/corelib/io/qfilesystemwatcher.cpp @@ -48,7 +48,6 @@ #include #include #include -#include #include #include diff --git a/src/corelib/io/qfilesystemwatcher_inotify.cpp b/src/corelib/io/qfilesystemwatcher_inotify.cpp index 80eb6303cc..49299e2254 100644 --- a/src/corelib/io/qfilesystemwatcher_inotify.cpp +++ b/src/corelib/io/qfilesystemwatcher_inotify.cpp @@ -245,8 +245,6 @@ QStringList QInotifyFileSystemWatcherEngine::addPaths(const QStringList &paths, QStringList *files, QStringList *directories) { - QMutexLocker locker(&mutex); - QStringList p = paths; QMutableListIterator it(p); while (it.hasNext()) { @@ -303,8 +301,6 @@ QStringList QInotifyFileSystemWatcherEngine::removePaths(const QStringList &path QStringList *files, QStringList *directories) { - QMutexLocker locker(&mutex); - QStringList p = paths; QMutableListIterator it(p); while (it.hasNext()) { @@ -331,8 +327,6 @@ QStringList QInotifyFileSystemWatcherEngine::removePaths(const QStringList &path void QInotifyFileSystemWatcherEngine::readFromInotify() { - QMutexLocker locker(&mutex); - // qDebug() << "QInotifyFileSystemWatcherEngine::readFromInotify"; int buffSize = 0; diff --git a/src/corelib/io/qfilesystemwatcher_inotify_p.h b/src/corelib/io/qfilesystemwatcher_inotify_p.h index 24fc88ffeb..daedb01fac 100644 --- a/src/corelib/io/qfilesystemwatcher_inotify_p.h +++ b/src/corelib/io/qfilesystemwatcher_inotify_p.h @@ -81,7 +81,6 @@ private Q_SLOTS: private: QInotifyFileSystemWatcherEngine(int fd); int inotifyFd; - QMutex mutex; QHash pathToID; QHash idToPath; QSocketNotifier notifier; diff --git a/src/corelib/io/qfilesystemwatcher_kqueue.cpp b/src/corelib/io/qfilesystemwatcher_kqueue.cpp index 1f16f6a226..8070af5e18 100644 --- a/src/corelib/io/qfilesystemwatcher_kqueue.cpp +++ b/src/corelib/io/qfilesystemwatcher_kqueue.cpp @@ -98,76 +98,72 @@ QStringList QKqueueFileSystemWatcherEngine::addPaths(const QStringList &paths, QStringList *directories) { QStringList p = paths; - { - QMutexLocker locker(&mutex); - - QMutableListIterator it(p); - while (it.hasNext()) { - QString path = it.next(); - int fd; + QMutableListIterator it(p); + while (it.hasNext()) { + QString path = it.next(); + int fd; #if defined(O_EVTONLY) - fd = qt_safe_open(QFile::encodeName(path), O_EVTONLY); + fd = qt_safe_open(QFile::encodeName(path), O_EVTONLY); #else - fd = qt_safe_open(QFile::encodeName(path), O_RDONLY); + fd = qt_safe_open(QFile::encodeName(path), O_RDONLY); #endif - if (fd == -1) { - perror("QKqueueFileSystemWatcherEngine::addPaths: open"); - continue; - } - if (fd >= (int)FD_SETSIZE / 2 && fd < (int)FD_SETSIZE) { - int fddup = fcntl(fd, F_DUPFD, FD_SETSIZE); - if (fddup != -1) { - ::close(fd); - fd = fddup; - } - } - fcntl(fd, F_SETFD, FD_CLOEXEC); - - QT_STATBUF st; - if (QT_FSTAT(fd, &st) == -1) { - perror("QKqueueFileSystemWatcherEngine::addPaths: fstat"); - ::close(fd); - continue; - } - int id = (S_ISDIR(st.st_mode)) ? -fd : fd; - if (id < 0) { - if (directories->contains(path)) { - ::close(fd); - continue; - } - } else { - if (files->contains(path)) { - ::close(fd); - continue; - } - } - - struct kevent kev; - EV_SET(&kev, - fd, - EVFILT_VNODE, - EV_ADD | EV_ENABLE | EV_CLEAR, - NOTE_DELETE | NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB | NOTE_RENAME | NOTE_REVOKE, - 0, - 0); - if (kevent(kqfd, &kev, 1, 0, 0, 0) == -1) { - perror("QKqueueFileSystemWatcherEngine::addPaths: kevent"); - ::close(fd); - continue; - } - - it.remove(); - if (id < 0) { - DEBUG() << "QKqueueFileSystemWatcherEngine: added directory path" << path; - directories->append(path); - } else { - DEBUG() << "QKqueueFileSystemWatcherEngine: added file path" << path; - files->append(path); - } - - pathToID.insert(path, id); - idToPath.insert(id, path); + if (fd == -1) { + perror("QKqueueFileSystemWatcherEngine::addPaths: open"); + continue; } + if (fd >= (int)FD_SETSIZE / 2 && fd < (int)FD_SETSIZE) { + int fddup = fcntl(fd, F_DUPFD, FD_SETSIZE); + if (fddup != -1) { + ::close(fd); + fd = fddup; + } + } + fcntl(fd, F_SETFD, FD_CLOEXEC); + + QT_STATBUF st; + if (QT_FSTAT(fd, &st) == -1) { + perror("QKqueueFileSystemWatcherEngine::addPaths: fstat"); + ::close(fd); + continue; + } + int id = (S_ISDIR(st.st_mode)) ? -fd : fd; + if (id < 0) { + if (directories->contains(path)) { + ::close(fd); + continue; + } + } else { + if (files->contains(path)) { + ::close(fd); + continue; + } + } + + struct kevent kev; + EV_SET(&kev, + fd, + EVFILT_VNODE, + EV_ADD | EV_ENABLE | EV_CLEAR, + NOTE_DELETE | NOTE_WRITE | NOTE_EXTEND | NOTE_ATTRIB | NOTE_RENAME | NOTE_REVOKE, + 0, + 0); + if (kevent(kqfd, &kev, 1, 0, 0, 0) == -1) { + perror("QKqueueFileSystemWatcherEngine::addPaths: kevent"); + ::close(fd); + continue; + } + + it.remove(); + if (id < 0) { + DEBUG() << "QKqueueFileSystemWatcherEngine: added directory path" << path; + directories->append(path); + } else { + DEBUG() << "QKqueueFileSystemWatcherEngine: added file path" << path; + files->append(path); + } + + pathToID.insert(path, id); + idToPath.insert(id, path); } return p; @@ -179,29 +175,26 @@ QStringList QKqueueFileSystemWatcherEngine::removePaths(const QStringList &paths { bool isEmpty; QStringList p = paths; - { - QMutexLocker locker(&mutex); - if (pathToID.isEmpty()) - return p; + if (pathToID.isEmpty()) + return p; - QMutableListIterator it(p); - while (it.hasNext()) { - QString path = it.next(); - int id = pathToID.take(path); - QString x = idToPath.take(id); - if (x.isEmpty() || x != path) - continue; + QMutableListIterator it(p); + while (it.hasNext()) { + QString path = it.next(); + int id = pathToID.take(path); + QString x = idToPath.take(id); + if (x.isEmpty() || x != path) + continue; - ::close(id < 0 ? -id : id); + ::close(id < 0 ? -id : id); - it.remove(); - if (id < 0) - directories->removeAll(path); - else - files->removeAll(path); - } - isEmpty = pathToID.isEmpty(); + it.remove(); + if (id < 0) + directories->removeAll(path); + else + files->removeAll(path); } + isEmpty = pathToID.isEmpty(); return p; } @@ -224,7 +217,6 @@ void QKqueueFileSystemWatcherEngine::readFromKqueue() int fd = kev.ident; DEBUG() << "QKqueueFileSystemWatcherEngine: processing kevent" << kev.ident << kev.filter; - QMutexLocker locker(&mutex); int id = fd; QString path = idToPath.value(id); diff --git a/src/corelib/io/qfilesystemwatcher_kqueue_p.h b/src/corelib/io/qfilesystemwatcher_kqueue_p.h index 6e36bcd8e5..41afbadf6a 100644 --- a/src/corelib/io/qfilesystemwatcher_kqueue_p.h +++ b/src/corelib/io/qfilesystemwatcher_kqueue_p.h @@ -85,7 +85,6 @@ private: int kqfd; - QMutex mutex; QHash pathToID; QHash idToPath; QSocketNotifier notifier; diff --git a/src/corelib/io/qfilesystemwatcher_polling.cpp b/src/corelib/io/qfilesystemwatcher_polling.cpp index 3d55c6d0aa..ef106acd0b 100644 --- a/src/corelib/io/qfilesystemwatcher_polling.cpp +++ b/src/corelib/io/qfilesystemwatcher_polling.cpp @@ -54,7 +54,6 @@ QStringList QPollingFileSystemWatcherEngine::addPaths(const QStringList &paths, QStringList *files, QStringList *directories) { - QMutexLocker locker(&mutex); QStringList p = paths; QMutableListIterator it(p); while (it.hasNext()) { @@ -89,7 +88,6 @@ QStringList QPollingFileSystemWatcherEngine::removePaths(const QStringList &path QStringList *files, QStringList *directories) { - QMutexLocker locker(&mutex); QStringList p = paths; QMutableListIterator it(p); while (it.hasNext()) { @@ -113,7 +111,6 @@ QStringList QPollingFileSystemWatcherEngine::removePaths(const QStringList &path void QPollingFileSystemWatcherEngine::timeout() { - QMutexLocker locker(&mutex); QMutableHashIterator fit(files); while (fit.hasNext()) { QHash::iterator x = fit.next(); diff --git a/src/corelib/io/qfilesystemwatcher_polling_p.h b/src/corelib/io/qfilesystemwatcher_polling_p.h index 1fae542561..717d438582 100644 --- a/src/corelib/io/qfilesystemwatcher_polling_p.h +++ b/src/corelib/io/qfilesystemwatcher_polling_p.h @@ -105,7 +105,6 @@ class QPollingFileSystemWatcherEngine : public QFileSystemWatcherEngine } }; - mutable QMutex mutex; QHash files, directories; public: