diff options
| author | Danilo M. <danix@danix.xyz> | 2026-08-25 11:33:16 +0200 |
|---|---|---|
| committer | Danilo M. <danix@danix.xyz> | 2026-08-25 11:33:16 +0200 |
| commit | 0c98e07afc848b5563263945c94076125e265f79 (patch) | |
| tree | 6a2cfef0d3f0ebafa9dd657299715773f3127f19 /src/notmuchworker.cpp | |
| parent | bfc5f78758f527f969204b61ad2317470a454e59 (diff) | |
| download | qtmaildir-0c98e07afc848b5563263945c94076125e265f79.tar.gz qtmaildir-0c98e07afc848b5563263945c94076125e265f79.zip | |
fix(delete): recover when a sync renamed the file underneath a move
Item 162. mbsync uploads a file and renames it to record the server UID
(`<name>,U=<uid>:2,<flags>`), and notmuch keeps the pre-`U=` name until
that sync's `notmuch new` runs. moveMessages() renamed a path that no
longer existed, reported "Cannot move <file> to <folder>", and skipped
the message: Delete silently did nothing while blaming the destination
folder for a timing problem.
Before renaming, check whether the recorded path still exists. If it
does not, reindex that one Maildir directory and re-read the message's
filenames by id, taking the one that is on disk.
Recovery is by MESSAGE ID rather than by scanning the folder, because
two files can carry the same id and scanning could move the wrong one.
reindexFolder() indexes a single directory and is deliberately not a
`notmuch new`, which would walk the whole Maildir and run the post-new
hook that tags real mail.
Bounded to one reindex and one retry, so a file that is genuinely gone
still reports rather than becoming a silent no-op. The second test pins
that half.
Holding the move while a sync runs was the other candidate and is not
the fix: sendMove() already refuses on the write lock (items 97 and
106), but aSyncHoldsTheWriteLock() tracks notmuch's lock, while this
window sits between mbsync's rename and that sync's `notmuch new`.
mbsync renames without touching that lock, so the damaging window is
open when there is nothing to observe. That refusal is left alone; it
does its own job.
The ordinary fixture layout cannot see this, since nothing renames a
file underneath the index. The test renames without reindexing, which
is exactly the window mbsync opens, and guards that the database still
names the old path so it cannot pass against a fixture that quietly
reindexed. Mutation-checked: disabling the recovery reproduces the
original error.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8
Diffstat (limited to 'src/notmuchworker.cpp')
| -rw-r--r-- | src/notmuchworker.cpp | 82 |
1 files changed, 81 insertions, 1 deletions
diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index 16df4ed..f510fd5 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -184,6 +184,39 @@ void walkReplies(notmuch_messages_t *messages, int depth, } } +/// Teach the database the current filenames in one Maildir directory. +/// +/// Item 162's recovery step. mbsync renames a file to add its `,U=<uid>` infix +/// and notmuch does not learn the new name until a `notmuch new` runs; this +/// indexes just the one directory rather than waiting for that sweep. +/// +/// Deliberately NOT a full `notmuch new`: that walks the entire Maildir and +/// runs the post-new hook, which tags real mail. This must stay a read of one +/// folder with no side effects beyond the filenames it records. +/// +/// Indexing a file already known under another name ADDS a filename to the +/// same message rather than creating a second message, which is what lets the +/// caller pick the surviving path out of get_filenames(). Errors are ignored +/// on purpose: this is a best-effort repair whose caller reports the failure +/// if the path is still missing afterwards. +void reindexFolder(notmuch_database_t *db, const QString &folder) +{ + const QDir dir(folder); + if (!dir.exists()) + return; + + const QFileInfoList entries = + dir.entryInfoList(QDir::Files | QDir::NoDotAndDotDot); + for (const QFileInfo &entry : entries) { + notmuch_message_t *indexed = nullptr; + notmuch_database_index_file( + db, entry.absoluteFilePath().toUtf8().constData(), nullptr, + &indexed); + if (indexed) + notmuch_message_destroy(indexed); + } +} + /// The Maildir FOLDER a message file sits in, relative to the database root. /// /// `<root>/acct/inbox/cur/12345` becomes `acct/inbox`: the `cur`/`new` segment @@ -747,7 +780,54 @@ void NotmuchWorker::moveMessages(const QStringList &messageIds, const char *rawName = notmuch_message_get_filename(message.get()); if (!rawName) continue; - const QString from = QString::fromUtf8(rawName); + QString from = QString::fromUtf8(rawName); + + // Item 162. mbsync renames an uploaded file to record the server UID + // (`<name>,U=<uid>:2,<flags>`), and notmuch keeps the pre-`U=` name + // until that sync's `notmuch new` runs. Renaming a path that no longer + // exists fails, and Delete silently does nothing while blaming the + // destination folder for a timing problem. + // + // Refusing while a sync holds the write lock does NOT close this: + // mbsync renames throughout its run without touching notmuch's lock, + // so the damaging window is open when there is nothing to observe. + // Re-resolving is what closes it. + // + // Recovery is by MESSAGE ID, never by scanning the folder: two files + // can carry the same id, and picking the wrong one moves the wrong + // file. notmuch_message_get_filenames() lists every path the database + // holds for this id, so a file that was renamed rather than removed is + // found among them once the folder is reindexed. + if (!QFileInfo::exists(from)) { + // One reindex of the containing folder, which is what teaches + // notmuch the new name. Bounded deliberately: a single attempt, + // and a message that is still missing afterwards falls through to + // the error below, so a genuinely deleted file is still reported + // (item 41's territory) rather than becoming a silent no-op. + const QString folder = QFileInfo(from).absolutePath(); + message.reset(); + reindexFolder(db, folder); + + notmuch_message_t *again = nullptr; + if (notmuch_database_find_message(db, id.toUtf8().constData(), + &again) + == NOTMUCH_STATUS_SUCCESS + && again) { + message.reset(again); + for (NmFilenames names( + notmuch_message_get_filenames(message.get())); + notmuch_filenames_valid(names.get()); + notmuch_filenames_move_to_next(names.get())) { + const QString candidate = QString::fromUtf8( + notmuch_filenames_get(names.get())); + if (QFileInfo::exists(candidate)) { + from = candidate; + break; + } + } + } + } + // The handle is released before the file moves under it. message.reset(); |
