diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-04 18:54:40 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-04 18:54:40 +0200 |
| commit | ce0753bcb6263dec5a6acb53647ca7b64950f8fd (patch) | |
| tree | a202a583a6f794c743cc838b33da2d1a79cfc07b /src/mainwindow.cpp | |
| parent | 61b85ed8a43971adba30be602902a3efb96c4ce7 (diff) | |
| download | qtmaildir-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/mainwindow.cpp')
| -rw-r--r-- | src/mainwindow.cpp | 34 |
1 files changed, 26 insertions, 8 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.")); } } |
