diff --git a/contrib/testing/upstream-imaptest-check.py b/contrib/testing/upstream-imaptest-check.py index 6552021..1b233e9 100755 --- a/contrib/testing/upstream-imaptest-check.py +++ b/contrib/testing/upstream-imaptest-check.py @@ -456,6 +456,10 @@ def stress(args: argparse.Namespace) -> None: raise UpstreamTestError( "stress run operated on a missing fixture mailbox" ) + if re.search(r"(?m)^Error:", output): + raise UpstreamTestError( + "Dovecot ImapTest reported a state-tracking error" + ) finally: time.sleep(0.1) cleanup_scripted_mailboxes(args, args.mailbox) diff --git a/src/agents/imap/event.c b/src/agents/imap/event.c index 18f5230..0177245 100644 --- a/src/agents/imap/event.c +++ b/src/agents/imap/event.c @@ -515,6 +515,7 @@ SendRememberedEvents(ImapSession *session, unsigned long typesAllowed) if ((events->remembered & STORE_EVENT_NEW) && (typesAllowed & STORE_EVENT_NEW)) { uint32_t recentUid; + BOOL recentLocked = !selectedFolder->readOnly; sequenceChanged = TRUE; /* @@ -523,22 +524,34 @@ SendRememberedEvents(ImapSession *session, unsigned long typesAllowed) * \Recent flags and advances the shared threshold; later sessions * therefore cannot report the same messages as recent. */ + if (recentLocked) FolderRecentLock(); ccode = FolderGetRecentUid(storeConn, selectedFolder->info, &recentUid); - if (ccode != STATUS_CONTINUE) return ccode; + if (ccode != STATUS_CONTINUE) { + if (recentLocked) FolderRecentUnlock(); + return ccode; + } selectedFolder->info->uidRecent = recentUid; ccode = MessageListAddNewMessages(storeConn, selectedFolder, events->new, events->newCount); - if (ccode != STATUS_CONTINUE) return ccode; + if (ccode != STATUS_CONTINUE) { + if (recentLocked) FolderRecentUnlock(); + return ccode; + } /* * A newly delivered message can already carry keywords. Announce * the expanded mailbox FLAGS set before any later FETCH can expose * such a keyword to this selected client. */ ccode = SendMailboxFlags(clientConn, selectedFolder); - if (ccode != STATUS_CONTINUE) return ccode; + if (ccode != STATUS_CONTINUE) { + if (recentLocked) FolderRecentUnlock(); + return ccode; + } if (!selectedFolder->readOnly) { ccode = FolderSetRecentUid(storeConn, selectedFolder->info); + FolderRecentUnlock(); + recentLocked = FALSE; if (ccode != STATUS_CONTINUE) return ccode; } events->newCount = 0; diff --git a/src/agents/imap/imapd.c b/src/agents/imap/imapd.c index 3b077bd..57299b1 100644 --- a/src/agents/imap/imapd.c +++ b/src/agents/imap/imapd.c @@ -333,6 +333,18 @@ FolderSetRecentUid(Connection *storeConn, FolderInformation *folder) return(CheckForNMAPCommError(ccode)); } +void +FolderRecentLock(void) +{ + XplMutexLock(Imap.recentLock); +} + +void +FolderRecentUnlock(void) +{ + XplMutexUnlock(Imap.recentLock); +} + __inline static long FolderWatchAdd(Connection *storeConn, FolderInformation *folder, const char *eventString) { @@ -368,9 +380,12 @@ FolderDeselect(ImapSession *session) ccode = FolderWatchRemove(session->store.conn, selectedFolder->info); EventsFree(&(selectedFolder->events)); - if ((ccode == STATUS_CONTINUE) && !(selectedFolder->readOnly)) { - ccode = FolderSetRecentUid(session->store.conn, selectedFolder->info); - } + /* + * A read-write SELECT and every NEW-event batch claim \Recent when + * they load it. Rewriting the threshold here from a stale + * session-local uidNext can move it backwards and let another + * session claim the same messages. + */ } FolderClose(selectedFolder); @@ -542,7 +557,10 @@ FolderSelect(ImapSession *session, char *folderName, BOOL readOnly) if ((ccode = FolderGetByName(session, folderName, &folder)) == STATUS_CONTINUE) { if ((ccode = FolderWatchAdd(session->store.conn, folder, "FLAGS MODIFIED NEW DELETED")) == STATUS_CONTINUE) { - if ((ccode = FolderOpen(session->store.conn, &session->folder.selected, folder, readOnly)) == STATUS_CONTINUE) { + if (!readOnly) FolderRecentLock(); + ccode = FolderOpen(session->store.conn, + &session->folder.selected, folder, readOnly); + if (ccode == STATUS_CONTINUE) { /* * A read-write SELECT claims the current \Recent set for this * session. Keep the session-local flags loaded by FolderOpen, @@ -552,15 +570,18 @@ FolderSelect(ImapSession *session, char *folderName, BOOL readOnly) if (!readOnly && (ccode = FolderSetRecentUid(session->store.conn, folder)) != STATUS_CONTINUE) { + FolderRecentUnlock(); FolderClose(&session->folder.selected); FolderWatchRemove(session->store.conn, folder); EventsFree(&session->folder.selected.events); return ccode; } + if (!readOnly) FolderRecentUnlock(); session->client.state = STATE_SELECTED; ImapSearchResultsReset(session); return(STATUS_CONTINUE); } + if (!readOnly) FolderRecentUnlock(); FolderWatchRemove(session->store.conn, folder); EventsFree(&(session->folder.selected.events)); } @@ -4465,6 +4486,7 @@ InitializeImapGlobals() /* Initialize the Busy List and Semaphore */ Imap.list_Busy = NULL; XplOpenLocalSemaphore(Imap.sem_Busy, 1); + XplMutexInit(Imap.recentLock); /* Global allocations */ BongoKeywordIndexCreateFromTable(Imap.command.index, ImapProtocolCommands, .name, TRUE); @@ -4502,6 +4524,7 @@ FreeImapGlobals() CommandFetchCleanup(); FreeReturnValueIndex(Imap.command.returnValueIndex); BongoKeywordIndexFree(Imap.command.index); + XplMutexDestroy(Imap.recentLock); return(TRUE); } diff --git a/src/agents/imap/imapd.h b/src/agents/imap/imapd.h index 1f1d193..4d05851 100755 --- a/src/agents/imap/imapd.h +++ b/src/agents/imap/imapd.h @@ -442,6 +442,7 @@ typedef struct { BongoList *list_Busy; /* Singly linked list of sessions that we should update every 10 seconds */ XplSemaphore sem_Busy; /* Semaphore protecting the busy list */ + XplMutex recentLock; /* serializes the shared \Recent claim */ void *logHandle; XplAtomic exiting; @@ -463,6 +464,8 @@ long GetMessageRange(MessageInformation *message, unsigned long messageCount, ch long FolderOpen(Connection *storeConn, OpenedFolder *openFolder, FolderInformation *folder, BOOL readOnly); long FolderListLoad(ImapSession *session); +void FolderRecentLock(void); +void FolderRecentUnlock(void); long FolderListInitialize(ImapSession *session); long MessageListLoad(Connection *conn, OpenedFolder *selected);