diff options
| -rw-r--r-- | src/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 41 | ||||
| -rw-r--r-- | src/mainwindow.h | 7 | ||||
| -rw-r--r-- | src/syncmonitor.cpp | 151 | ||||
| -rw-r--r-- | src/syncmonitor.h | 106 | ||||
| -rw-r--r-- | tests/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | tests/test_syncmonitor.cpp | 180 |
7 files changed, 487 insertions, 0 deletions
diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 5214f42..a71a6f1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -12,6 +12,7 @@ add_library(qtmaildir_lib STATIC tagstrip.cpp threadlistmodel.cpp mailsync.cpp + syncmonitor.cpp threadcidmap.cpp messageview.cpp mainwindow.cpp diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index b3b67c5..7d0496d 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -431,6 +431,15 @@ void MainWindow::buildUi() m_syncLog->appendPlainText(chunk.trimmed()); }); + // Syncs this window did not start. The user's cron runs the same script + // every ten minutes, so mail arrives and tags change while the window sits + // idle, and until now nothing here noticed. + m_syncMonitor = new SyncMonitor(SyncMonitor::defaultLockPath(), + QStringLiteral("/proc/locks"), this); + connect(m_syncMonitor, &SyncMonitor::stateChanged, + this, &MainWindow::onExternalSyncStateChanged); + m_syncMonitor->start(); + queryRow->addWidget(m_accountBox); queryRow->addWidget(m_queryEdit, 1); queryRow->addWidget(m_syncButton); @@ -1289,6 +1298,38 @@ void MainWindow::onTagsApplied(const TagChange &change) } } +void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state) +{ + // A sync this window started is already reported by setSyncBusy(), and the + // monitor sees that lock too. Saying so twice would fight over the status + // bar and would re-enable the progress bar as the local run finished. + if (m_sync && m_sync->isRunning()) + return; + + if (state == SyncMonitor::State::Running) { + m_syncProgress->setVisible(true); + m_statusLabel->setText(tr("Syncing (started elsewhere)...")); + return; + } + + m_syncProgress->setVisible(false); + + // Deliberately reports rather than refreshes. runCurrentQuery() clears the + // undo stack, the selection and the message pane, which is right for a + // query the user typed and hostile for one fired by a cron timer: with a + // sync every ten minutes it would discard undo history and close the thread + // being read, up to six times an hour, with no action from the user. + // + // Unknown is not worth reporting either. It means the lock table could not + // be read, so nothing was observed, and "sync finished" would be a claim + // this cannot support. + if (state == SyncMonitor::State::Idle) { + m_statusLabel->setText( + tr("Sync finished elsewhere. Press Enter in the query bar to " + "refresh.")); + } +} + void MainWindow::setSyncBusy(bool busy) { m_syncProgress->setVisible(busy); diff --git a/src/mainwindow.h b/src/mainwindow.h index 933c71b..57aceb1 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -28,6 +28,7 @@ #include "config.h" #include "keymap.h" +#include "syncmonitor.h" #include "tagcolors.h" #include "types.h" @@ -168,6 +169,9 @@ private: /// says "working, duration unknown", which is the truth. void setSyncBusy(bool busy); + /// Reacts to a sync started outside this window, by cron or by hand. + void onExternalSyncStateChanged(SyncMonitor::State state); + /// Opens the tag dialog on the current selection and applies its result. /// /// The only route to an arbitrary tag: every other tag action writes a @@ -206,6 +210,9 @@ private: ThreadListModel *m_model = nullptr; MessageView *m_messageView = nullptr; MailSync *m_sync = nullptr; + + /// Watches the sync lock for runs this window did not start. + SyncMonitor *m_syncMonitor = nullptr; QUndoStack m_undoStack; QLineEdit *m_queryEdit = nullptr; diff --git a/src/syncmonitor.cpp b/src/syncmonitor.cpp new file mode 100644 index 0000000..e2883dc --- /dev/null +++ b/src/syncmonitor.cpp @@ -0,0 +1,151 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include "syncmonitor.h" + +#include <QFile> +#include <QFileInfo> + +#include <sys/stat.h> + +namespace { + +/// Two seconds. The user chose continuous polling over a poll-only-while- +/// quitting variant, and this is a read of one small procfs file, so the cost +/// is negligible next to noticing a cron sync within a couple of seconds. +constexpr int kDefaultIntervalMs = 2000; + +} // namespace + +SyncMonitor::SyncMonitor(const QString &lockPath, const QString &locksPath, + QObject *parent) + : QObject(parent), m_lockPath(lockPath), m_locksPath(locksPath) +{ + m_timer.setInterval(kDefaultIntervalMs); + connect(&m_timer, &QTimer::timeout, this, &SyncMonitor::poll); +} + +void SyncMonitor::setInterval(int ms) +{ + m_timer.setInterval(ms); +} + +void SyncMonitor::start() +{ + // Poll once immediately: a window opened during a cron sync should say so + // at once rather than after the first interval. + poll(); + m_timer.start(); +} + +void SyncMonitor::stop() +{ + m_timer.stop(); +} + +QString SyncMonitor::defaultLockPath() +{ + // Must stay equal to LOCKFILE in assets/mailsync.sh. + return QStringLiteral("/tmp/mbsync.lock"); +} + +qint64 SyncMonitor::inodeOf(const QString &path) +{ + // Qt exposes no inode accessor, and /proc/locks identifies a file only by + // device and inode, so this has to come from stat(2) directly. That is also + // why the whole class is Linux-shaped; see the Unknown state for what + // happens where /proc/locks does not exist. + struct stat st; + if (::stat(QFile::encodeName(path).constData(), &st) != 0) + return -1; + + return static_cast<qint64>(st.st_ino); +} + +bool SyncMonitor::lockHeldIn(const QString &content, qint64 inode) +{ + if (content.isEmpty() || inode < 0) + return false; + + // A /proc/locks line looks like: + // 82: FLOCK ADVISORY WRITE 9051 fc:00:12058676 0 EOF + // The inode is the last colon-separated part of the major:minor:inode + // field. Matching the raw number anywhere in the line would also match a + // pid or a byte range, so the field is located first and then split. + const QList<QStringView> lines = QStringView(content).split(u'\n', + Qt::SkipEmptyParts); + for (const QStringView &line : lines) { + const QList<QStringView> fields = + line.split(u' ', Qt::SkipEmptyParts); + + // Shortest real line still has: index, type, ADVISORY, WRITE, pid, + // dev:inode. Anything shorter is truncated or not a lock line, and is + // skipped rather than guessed at. + if (fields.size() < 6) + continue; + + // flock(2) only. mailsync.sh uses flock, and a POSIX record lock on + // the same file belongs to somebody else: the two namespaces cannot + // see each other, so treating a POSIX entry as ours would report a + // sync that is not running. + if (fields.at(1) != QLatin1String("FLOCK")) + continue; + + for (const QStringView &field : fields) { + const qsizetype lastColon = field.lastIndexOf(u':'); + if (lastColon < 0) + continue; + + bool ok = false; + const qint64 candidate = + field.mid(lastColon + 1).toLongLong(&ok); + if (ok && candidate == inode) + return true; + } + } + + return false; +} + +void SyncMonitor::poll() +{ + State next = State::Unknown; + + QFile locks(m_locksPath); + if (locks.open(QIODevice::ReadOnly | QIODevice::Text)) { + // Read in full rather than line by line: /proc/locks is small, and a + // partial read while the kernel is editing the table could truncate a + // line mid-field. + const QString content = QString::fromUtf8(locks.readAll()); + + const qint64 inode = inodeOf(m_lockPath); + if (inode < 0) { + // No lock file yet, before the first sync ever runs. The table was + // readable, so this is a real answer and not Unknown. + next = State::Idle; + } else { + next = lockHeldIn(content, inode) ? State::Running : State::Idle; + } + } + + if (next == m_state) + return; + + m_state = next; + emit stateChanged(m_state); +} diff --git a/src/syncmonitor.h b/src/syncmonitor.h new file mode 100644 index 0000000..3aca520 --- /dev/null +++ b/src/syncmonitor.h @@ -0,0 +1,106 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#pragma once + +#include <QObject> +#include <QString> +#include <QTimer> + +/// Notices syncs this process did not start. +/// +/// The user's cron runs mailsync.sh every ten minutes, so mail can appear and +/// tags can change while the window sits idle. The script holds an flock for +/// the whole run, which is already the signal: no status file is needed, and a +/// kernel lock cannot go stale because it dies with the process holding it. +/// +/// **Read the lock, never take it.** Three ways to observe an flock look +/// plausible and two are wrong, both verified on Slackware, Linux 6.18: +/// +/// - `flock -n` acquires in order to test. Polling every two seconds would +/// open a window every two seconds in which a starting mailsync.sh is +/// refused the lock and exits 75. It would cause the very skips the sync +/// script reports. +/// - `fcntl(F_OFD_GETLK)` never acquires, and looks ideal, but reports +/// UNLOCKED against a lock held by flock(2): the two are separate lock +/// namespaces in the kernel and cannot see each other. A silent false +/// negative, which is the worst failure available here. +/// - /proc/locks is a pure read. It observes flock(2) entries correctly and +/// cannot acquire, steal, or contend, so it can also never disturb the +/// Xapian write lock notmuch new holds during the same run. +/// +/// Do not "simplify" this to flock -n. +class SyncMonitor : public QObject +{ + Q_OBJECT +public: + enum class State { + Unknown, ///< The lock table cannot be read; claim nothing. + Idle, ///< Readable, and nothing holds the lock. + Running, ///< Something holds the lock: a sync is in progress. + }; + Q_ENUM(State) + + /// @param lockPath the file mailsync.sh flocks, /tmp/mbsync.lock. + /// @param locksPath the kernel lock table; injectable so tests can drive + /// transitions without holding real locks. + explicit SyncMonitor(const QString &lockPath, + const QString &locksPath = QStringLiteral("/proc/locks"), + QObject *parent = nullptr); + + State state() const { return m_state; } + + /// True only for State::Running. Unknown is deliberately not "running": + /// callers use this to decide whether to wait, and waiting forever on a + /// platform with no /proc/locks would be worse than not noticing a sync. + bool isRunning() const { return m_state == State::Running; } + + void setInterval(int ms); + void start(); + void stop(); + + /// One observation. Public so tests can step it without a running timer. + void poll(); + + /// Whether @p content holds an flock(2) entry for @p inode. + /// + /// Static and content-based: this is the part worth testing, and it is + /// testable only while it is separate from reading the file. + static bool lockHeldIn(const QString &content, qint64 inode); + + /// The inode of @p path, or -1 when it does not exist. + static qint64 inodeOf(const QString &path); + + /// The lock file assets/mailsync.sh takes, and the only one worth watching. + /// + /// Hardcoded to match LOCKFILE in that script. Two sources of truth is the + /// standing hazard here: change one and the monitor silently reports Idle + /// forever, since a missing lock file is a legitimate "no sync running". + static QString defaultLockPath(); + +signals: + /// Emitted only when the state actually changes, never once per poll: the + /// status bar must not be repainted every two seconds forever. + void stateChanged(SyncMonitor::State state); + +private: + QString m_lockPath; + QString m_locksPath; + State m_state = State::Unknown; + QTimer m_timer; +}; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 7f7caa5..0f6ec5a 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -16,6 +16,7 @@ add_qtmaildir_test(notmuchworker) add_qtmaildir_test(tagcolors) add_qtmaildir_test(threadlistmodel) add_qtmaildir_test(mailsync) +add_qtmaildir_test(syncmonitor) add_qtmaildir_test(threadcidmap) add_qtmaildir_test(mainwindow) add_qtmaildir_test(messageview) diff --git a/tests/test_syncmonitor.cpp b/tests/test_syncmonitor.cpp new file mode 100644 index 0000000..c120085 --- /dev/null +++ b/tests/test_syncmonitor.cpp @@ -0,0 +1,180 @@ +/* + * qtmaildir - a Qt6 mail client for notmuch-indexed Maildirs + * Copyright (C) 2026 Danilo M. <danix@danix.xyz> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + */ + +#include <QtTest> + +#include <QSignalSpy> +#include <QTemporaryDir> +#include <QTemporaryFile> + +#include "syncmonitor.h" + +/// SyncMonitor answers one question: is a sync running that this process did +/// not start? The parsing of /proc/locks is the testable part and is kept +/// separate from the polling for exactly that reason. +class TestSyncMonitor : public QObject +{ + Q_OBJECT +private slots: + void anFlockOnTheWatchedInodeIsHeld(); + void anFlockOnAnotherInodeIsIgnored(); + void aPosixLockOnTheWatchedInodeIsIgnored(); + void emptyContentMeansNotHeld(); + void garbageLinesAreSkippedRatherThanMisread(); + void anUnreadableLockTableIsUnknownNotIdle(); + void aMissingLockFileIsNotHeld(); + void theStateChangeSignalFiresOnlyOnTransitions(); +}; + +void TestSyncMonitor::anFlockOnTheWatchedInodeIsHeld() +{ + // The real shape of a held lock, taken verbatim from /proc/locks while + // mailsync.sh held /tmp/mbsync.lock: the inode is the last colon-separated + // field of the device:inode column, not the whole column. + const QString content = + QStringLiteral("82: FLOCK ADVISORY WRITE 9051 fc:00:12058676 0 EOF\n"); + QVERIFY(SyncMonitor::lockHeldIn(content, 12058676)); +} + +void TestSyncMonitor::anFlockOnAnotherInodeIsIgnored() +{ + // A busy machine has many flocks. Matching anything but our own inode would + // report a sync whenever some unrelated program took a lock. + const QString content = + QStringLiteral("82: FLOCK ADVISORY WRITE 9051 fc:00:99999999 0 EOF\n"); + QVERIFY(!SyncMonitor::lockHeldIn(content, 12058676)); +} + +void TestSyncMonitor::aPosixLockOnTheWatchedInodeIsIgnored() +{ + // flock(2) and fcntl(2) are separate namespaces in the kernel and cannot + // see each other; mailsync.sh uses flock(2). A POSIX lock on the same file + // is somebody else's, and treating it as ours would report a sync that is + // not running. Verified: fcntl(F_OFD_GETLK) reports UNLOCKED against a + // held flock, which is why this distinction is not academic. + const QString content = + QStringLiteral("1: POSIX ADVISORY WRITE 1234 fc:00:12058676 0 EOF\n"); + QVERIFY(!SyncMonitor::lockHeldIn(content, 12058676)); +} + +void TestSyncMonitor::emptyContentMeansNotHeld() +{ + QVERIFY(!SyncMonitor::lockHeldIn(QString(), 12058676)); +} + +void TestSyncMonitor::garbageLinesAreSkippedRatherThanMisread() +{ + // /proc/locks gains fields across kernel versions, and a line can be + // truncated as it is read. A short line must not match by accident, and + // must not stop the lines after it from being read. + const QString content = QStringLiteral( + "not a lock line at all\n" + "3: FLOCK\n" + "82: FLOCK ADVISORY WRITE 9051 fc:00:12058676 0 EOF\n"); + QVERIFY(SyncMonitor::lockHeldIn(content, 12058676)); + + const QString onlyGarbage = QStringLiteral("nonsense\n3: FLOCK\n"); + QVERIFY(!SyncMonitor::lockHeldIn(onlyGarbage, 12058676)); +} + +void TestSyncMonitor::anUnreadableLockTableIsUnknownNotIdle() +{ + // /proc/locks is Linux-only. Where it cannot be read the honest answer is + // "unknown", and the indicator stays hidden. Reporting idle would be a + // claim the monitor cannot support, and it is the claim that matters: + // "no sync is running" is what lets the window quit. + SyncMonitor monitor(QStringLiteral("/nonexistent/mbsync.lock"), + QStringLiteral("/nonexistent/proc/locks")); + QCOMPARE(monitor.state(), SyncMonitor::State::Unknown); + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Unknown); +} + +void TestSyncMonitor::aMissingLockFileIsNotHeld() +{ + // Before the first sync ever runs there is no lock file. That is not a + // sync in progress, and it must not read as unknown either: the lock table + // is perfectly readable, there is simply nothing holding anything. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + + QTemporaryFile locks; + QVERIFY(locks.open()); + locks.write("82: FLOCK ADVISORY WRITE 9051 fc:00:12058676 0 EOF\n"); + locks.flush(); + + SyncMonitor monitor(dir.filePath(QStringLiteral("never-created.lock")), + locks.fileName()); + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Idle); +} + +void TestSyncMonitor::theStateChangeSignalFiresOnlyOnTransitions() +{ + // The UI reacts to a sync starting and finishing, so a signal on every + // poll would repaint the status bar every two seconds forever and would + // stamp over whatever else had been written there. + QTemporaryDir dir; + QVERIFY(dir.isValid()); + const QString lockPath = dir.filePath(QStringLiteral("mbsync.lock")); + { + QFile lock(lockPath); + QVERIFY(lock.open(QIODevice::WriteOnly)); + } + + const qint64 inode = SyncMonitor::inodeOf(lockPath); + QVERIFY(inode > 0); + + // A stand-in for /proc/locks whose contents the test controls. + const QString locksPath = dir.filePath(QStringLiteral("locks")); + auto writeLocks = [&locksPath](const QString &text) { + QFile f(locksPath); + QVERIFY(f.open(QIODevice::WriteOnly | QIODevice::Truncate)); + f.write(text.toUtf8()); + }; + writeLocks(QString()); + + SyncMonitor monitor(lockPath, locksPath); + QSignalSpy spy(&monitor, &SyncMonitor::stateChanged); + + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Idle); + QCOMPARE(spy.count(), 1); // Unknown -> Idle is a real transition. + + monitor.poll(); + monitor.poll(); + QCOMPARE(spy.count(), 1); // Still idle: no further signals. + + writeLocks(QStringLiteral("82: FLOCK ADVISORY WRITE 9051 fc:00:%1 0 EOF\n") + .arg(inode)); + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Running); + QCOMPARE(spy.count(), 2); + + monitor.poll(); + QCOMPARE(spy.count(), 2); // Still running. + + writeLocks(QString()); + monitor.poll(); + QCOMPARE(monitor.state(), SyncMonitor::State::Idle); + QCOMPARE(spy.count(), 3); +} + +QTEST_MAIN(TestSyncMonitor) + +#include "test_syncmonitor.moc" |