diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-04 18:25:37 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-04 18:25:37 +0200 |
| commit | 61b85ed8a43971adba30be602902a3efb96c4ce7 (patch) | |
| tree | 22ae46a4121c7af22256d73b61b63fc2e3f32009 /src | |
| parent | a86de2d924d68451c1db05c3055affe4eb654464 (diff) | |
| download | qtmaildir-61b85ed8a43971adba30be602902a3efb96c4ce7.tar.gz qtmaildir-61b85ed8a43971adba30be602902a3efb96c4ce7.zip | |
feat(sync): notice syncs this window did not start
The user's cron runs mailsync.sh every ten minutes, so mail arrives and
tags change while the window sits idle, and nothing here noticed. The
script already holds an flock for the whole run, so that lock is the
signal: no status file is needed, and a kernel lock cannot go stale
because it dies with the process holding it.
The observation method is the part that matters, and two of the three
plausible ones are wrong. Both were probed on Linux 6.18 before any of
this was written:
- flock -n acquires in order to test, so 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 script reports.
- fcntl(F_OFD_GETLK) never acquires and looks ideal, but reports
UNLOCKED against a lock held by flock(2): separate lock namespaces
in the kernel, which cannot see each other. A silent false negative.
- /proc/locks is a pure read. It observes flock(2) correctly, and
since it takes no lock at all it can never contend with the Xapian
write lock notmuch new holds during the same run. Confirmed: 200
reads left the lock table unchanged and this process holding
nothing.
SyncMonitor keeps the parsing separate from the polling so the parsing
is testable, and it reports Unknown rather than Idle where /proc/locks
cannot be read: "no sync is running" is the claim that would let the
window quit, so it must never be guessed. Verified against a real flock
end to end, not only against synthetic content.
It 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 a cron timer fired: it would discard
undo history and close the thread being read up to six times an hour,
with no action from the user.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src')
| -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 |
5 files changed, 306 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; +}; |
