aboutsummaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-04 18:54:40 +0200
committerDanilo M. <danix@danix.xyz>2026-08-04 18:54:40 +0200
commitce0753bcb6263dec5a6acb53647ca7b64950f8fd (patch)
treea202a583a6f794c743cc838b33da2d1a79cfc07b /src
parent61b85ed8a43971adba30be602902a3efb96c4ce7 (diff)
downloadqtmaildir-ce0753bcb6263dec5a6acb53647ca7b64950f8fd.tar.gz
qtmaildir-ce0753bcb6263dec5a6acb53647ca7b64950f8fd.zip
fix(sync): stop a local sync reporting itself as a background one
Reported from hand testing: a manual sync ended with "Sync finished elsewhere" stamped over its own result. Ownership of a lock period was being decided when the lock was RELEASED, by asking MailSync::isRunning(). That question cannot be answered then: the process exits, so isRunning() goes false, and only afterwards does the next poll observe the lock gone. The guard therefore suppressed the message while the sync ran and let it through at the end, up to two seconds after onSyncFinished() had already said what happened. Ownership is now latched when the lock APPEARS, which is the moment isRunning() can still answer, and the matching release is swallowed. onSyncFinished() hands the latch back when it sees exit 75, because a skip means the lock was never ours: if a manual run and the cron run start inside one poll interval, the lock would otherwise be latched as local and that other run's completion swallowed with it. Also renames the messages to "Background sync running/completed" per the user: "finished elsewhere" reads as though the application does not know what is syncing the Maildir, when in fact it is the same script. The tests added here cover the external path and the Unknown state. They do NOT reproduce the reported bug, and were checked against a reverted fix to confirm that: staging it needs isRunning() true at the Running transition and false at the Idle one, which cannot be arranged in test_mainwindow without a configured sync command and a live child process. That was tried and abandoned, it left a process running for the length of the suite and popped a dialog. The ordering and the fix were instead verified against a standalone model of both code paths. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'src')
-rw-r--r--src/mainwindow.cpp34
-rw-r--r--src/mainwindow.h14
2 files changed, 38 insertions, 10 deletions
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index 7d0496d..792ba3f 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -1240,6 +1240,12 @@ void MainWindow::onSyncFinished(bool success, int exitCode)
// A sync is the usual way new tags enter the database.
requestAllTags();
} else if (exitCode == kSyncSkippedExitCode) {
+ // Skipped means the lock was never ours: some other run holds it. If
+ // both started inside the same poll interval the monitor will have
+ // latched this lock period as local, which would swallow the report
+ // when that other run finishes. Hand it back.
+ m_localSyncHoldsLock = false;
+
// Not a failure: another run holds the lock and is doing the work.
// The user's cron fires every ten minutes, so a click landing inside
// one is routine and must not raise an error or the log pane.
@@ -1300,15 +1306,27 @@ 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) {
+ // A sync this window started is already reported by setSyncBusy().
+ // Remember that this particular lock period is ours, because the
+ // release at the end of it must be ignored too: the process exits, and
+ // therefore isRunning() goes false, BEFORE the monitor's next poll sees
+ // the lock gone. Testing isRunning() again on that poll would report a
+ // local sync as an external one, stamping "background sync completed"
+ // over the local run's own result up to two seconds later.
+ m_localSyncHoldsLock = (m_sync && m_sync->isRunning());
+ if (m_localSyncHoldsLock)
+ return;
+
m_syncProgress->setVisible(true);
- m_statusLabel->setText(tr("Syncing (started elsewhere)..."));
+ m_statusLabel->setText(tr("Background sync running..."));
+ return;
+ }
+
+ // The release of a lock this window took. onSyncFinished() has already
+ // said what happened, including for a failure, so there is nothing to add.
+ if (m_localSyncHoldsLock) {
+ m_localSyncHoldsLock = false;
return;
}
@@ -1325,7 +1343,7 @@ void MainWindow::onExternalSyncStateChanged(SyncMonitor::State state)
// this cannot support.
if (state == SyncMonitor::State::Idle) {
m_statusLabel->setText(
- tr("Sync finished elsewhere. Press Enter in the query bar to "
+ tr("Background sync completed. Press Enter in the query bar to "
"refresh."));
}
}
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 57aceb1..66044dc 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -115,6 +115,12 @@ private slots:
void onWorkerError(const QString &message);
void onSyncFinished(bool success, int exitCode);
+ /// Reacts to a sync started outside this window, by cron or by hand.
+ ///
+ /// A private slot rather than a plain method so tests can drive it through
+ /// the meta-object without widening the public API.
+ void onExternalSyncStateChanged(SyncMonitor::State state);
+
/// A tag mutation the worker has confirmed reached the database. Counts it
/// as unsynced, since reaching the index is not reaching the mail store.
void onTagsApplied(const TagChange &change);
@@ -169,8 +175,6 @@ 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.
///
@@ -213,6 +217,12 @@ private:
/// Watches the sync lock for runs this window did not start.
SyncMonitor *m_syncMonitor = nullptr;
+
+ /// True while the lock the monitor can see is held by this window's own
+ /// sync. Latched when the lock is taken, because by the time it is released
+ /// MailSync::isRunning() is already false and can no longer answer "was
+ /// that ours?".
+ bool m_localSyncHoldsLock = false;
QUndoStack m_undoStack;
QLineEdit *m_queryEdit = nullptr;