Problem with flushseenstate
David Carter
dpc22 at cam.ac.uk
Wed Feb 14 10:26:15 EST 2007
I believe that I have found a problem with the flushseenstate code in 2.3.
RFC 3501, section 7.4.1 states:
An EXPUNGE response MUST NOT be sent when no command is in
progress, nor while responding to a FETCH, STORE, or SEARCH
command. This rule is necessary to prevent a loss of synchronization
of message sequence numbers between client and server.
Here are two IMAP sessions, labelled A and B looking at a single mailbox:
EXPUNGE response to FETCH command:
A: . expunge
A: * 30 EXPUNGE
A: * 36 EXISTS
A: * 0 RECENT
A: . OK Completed
B: . fetch 32 fast
B: * 30 EXPUNGE <-- Not allowed.
B: * 36 EXISTS
B: * 0 RECENT
B: * 32 FETCH (FLAGS () INTERNALDATE
" 6-Jul-2004 17:17:05 +0100" RFC822.SIZE 2297)
B: . OK Completed (0.000 sec)
EXPUNGE response to STORE command:
A: . expunge
A: * 30 EXPUNGE
A: * 35 EXISTS
A: * 0 RECENT
A: . OK Completed
B: . store 32 flags (fred) <-- STOREs data for new message 31
B: * 32 FETCH (FLAGS (fred))
B: * 30 EXPUNGE
B: * 35 EXISTS
B: * 0 RECENT
B: . OK Completed
B: . fetch 31:32 fast
B: * 31 FETCH (FLAGS (fred) INTERNALDATE
" 6-Jul-2004 17:17:05 +0100" RFC822.SIZE 2297)
B: * 32 FETCH (FLAGS () INTERNALDATE
" 8-Apr-2005 08:09:46 +0100" RFC822.SIZE 494)
B: . OK Completed (0.000 sec)
Here is a patch which uses a new function called index_check_existing() to
generate unsolicited responses without reopening cyrus.index. It is
effectively a half way house between the standard behaviour and the
existing flushseenstate, but seems good enough to keep Outlook happy.
--
David Carter Email: David.Carter at ucs.cam.ac.uk
University Computing Service, Phone: (01223) 334502
New Museums Site, Pembroke Street, Fax: (01223) 334679
Cambridge UK. CB2 3QH.
-------------- next part --------------
Index: imap/imapd.c
===================================================================
RCS file: /cvs/src/cyrus/imap/imapd.c,v
retrieving revision 1.510
diff -u -r1.510 imapd.c
--- imap/imapd.c 13 Feb 2007 17:04:27 -0000 1.510
+++ imap/imapd.c 14 Feb 2007 14:52:45 -0000
@@ -4006,16 +4006,22 @@
imapd_mailbox->highestmodseq);
}
- if (usinguid || config_getswitch(IMAPOPT_FLUSHSEENSTATE)) {
- if (usinguid) fetchitems |= FETCH_UID;
- index_check(imapd_mailbox, usinguid, /* update \Seen state from disk? */
- config_getswitch(IMAPOPT_FLUSHSEENSTATE) << 1 /* quiet */);
+ /* EXPUNGE responses allowed for UID FETCH */
+ if (usinguid) {
+ fetchitems |= FETCH_UID;
+ index_check(imapd_mailbox, 1, 0);
}
fetchargs.fetchitems = fetchitems;
r = index_fetch(imapd_mailbox, sequence, usinguid, &fetchargs,
&fetchedsomething);
+ if (config_getswitch(IMAPOPT_FLUSHSEENSTATE)) {
+ /* Flush \Seen state to disk immediately (but only if anything
+ changed) and check for updates by other processes */
+ index_check_existing(imapd_mailbox, usinguid, 1);
+ }
+
snprintf(mytime, sizeof(mytime), "%2.3f",
(clock() - start) / (double) CLOCKS_PER_SEC);
@@ -4025,11 +4031,6 @@
} else if (fetchedsomething || usinguid) {
prot_printf(imapd_out, "%s OK %s (%s sec)\r\n", tag,
error_message(IMAP_OK_COMPLETED), mytime);
- if (config_getswitch(IMAPOPT_FLUSHSEENSTATE) &&
- (fetchargs.fetchitems & FETCH_SETSEEN)) {
- /* flush \Seen state to disk */
- index_check(imapd_mailbox, usinguid, 2 /* quiet */);
- }
} else {
/* normal FETCH, nothing came back */
prot_printf(imapd_out, "%s NO %s (%s sec)\r\n", tag,
@@ -4157,9 +4158,11 @@
index_fetch(imapd_mailbox, msgno, 0, &fetchargs, &fetchedsomething);
- index_check(imapd_mailbox, 0, /* flush \Seen state to disk? */
- config_getswitch(IMAPOPT_FLUSHSEENSTATE) &&
- fetchedsomething && (fetchargs.fetchitems & FETCH_SETSEEN));
+ if (config_getswitch(IMAPOPT_FLUSHSEENSTATE)) {
+ /* Flush \Seen state to disk immediately (but only if anything
+ changed) and check for updates by other processes */
+ index_check_existing(imapd_mailbox, 0, 1);
+ }
if (fetchedsomething) {
prot_printf(imapd_out, "%s OK %s\r\n", tag,
@@ -4369,13 +4372,14 @@
r = index_store(imapd_mailbox, sequence, usinguid, &storeargs,
flag, nflags);
- if (config_getswitch(IMAPOPT_FLUSHSEENSTATE) &&
- (storeargs.seen || storeargs.operation == STORE_REPLACE)) {
- /* flush \Seen state to disk */
- index_check(imapd_mailbox, usinguid, 1);
- }
- else if (usinguid) {
- index_check(imapd_mailbox, 1, 0);
+ if (usinguid) {
+ /* EXPUNGE responses allowed */
+ index_check(imapd_mailbox, 1, 1); /* Check \Seen too */
+ } else if (config_getswitch(IMAPOPT_FLUSHSEENSTATE)) {
+ /* EXPUNGE responses not allowed */
+ /* Flush \Seen state to disk immediately (but only if anything
+ changed) and check for updates by other processes */
+ index_check_existing(imapd_mailbox, 0, 1);
}
if (r) {
Index: imap/imapd.h
===================================================================
RCS file: /cvs/src/cyrus/imap/imapd.h,v
retrieving revision 1.63
diff -u -r1.63 imapd.h
--- imap/imapd.h 30 Nov 2006 17:11:18 -0000 1.63
+++ imap/imapd.h 14 Feb 2007 14:52:45 -0000
@@ -241,6 +241,8 @@
extern void index_operatemailbox(struct mailbox *mailbox);
extern void index_check(struct mailbox *mailbox, int usinguid,
int checkseen);
+extern void index_check_existing(struct mailbox *mailbox,
+ int usinguid, int checkseen);
extern void index_checkseen(struct mailbox *mailbox, int quiet,
int usinguid, int oldexists);
Index: imap/index.c
===================================================================
RCS file: /cvs/src/cyrus/imap/index.c,v
retrieving revision 1.223
diff -u -r1.223 index.c
--- imap/index.c 5 Feb 2007 18:41:46 -0000 1.223
+++ imap/index.c 14 Feb 2007 14:52:45 -0000
@@ -460,6 +460,54 @@
}
}
+/* Flush seen state (but only if anything changed) and check for flag/seen
+ * updates from other processes. Bails out if the cyrus.index file has
+ * changed under our feet, which indicates an expunge from another process.
+ * In this case pending updates will be flushed on the next index_check()
+ */
+void
+index_check_existing(struct mailbox *mailbox, int usinguid, int checkseen)
+{
+ struct stat sbuf;
+ int msgno, i;
+ bit32 user_flags[MAX_USER_FLAGS/32];
+
+ if (imapd_exists == -1)
+ return;
+
+ /* Check for expunge, just like index_check() */
+ if (index_len) {
+ char fnamebuf[MAX_MAILBOX_PATH+1], *path;
+
+ path = (mailbox->mpath &&
+ (config_metapartition_files &
+ IMAP_ENUM_METAPARTITION_FILES_INDEX)) ?
+ mailbox->mpath : mailbox->path;
+ strlcpy(fnamebuf, path, sizeof(fnamebuf));
+ strlcat(fnamebuf, FNAME_INDEX, sizeof(fnamebuf));
+
+ if ((stat(fnamebuf, &sbuf) != 0) ||
+ (sbuf.st_ino != mailbox->index_ino) ||
+ (index_ino != mailbox->index_ino))
+ return;
+ }
+
+ if (checkseen)
+ index_checkseen(mailbox, 0, usinguid, imapd_exists);
+
+ for (msgno = 1; msgno <= imapd_exists; msgno++) {
+ if (flagreport[msgno] < LAST_UPDATED(msgno)) {
+ for (i = 0; i < VECTOR_SIZE(user_flags); i++) {
+ user_flags[i] = USER_FLAGS(msgno, i);
+ }
+ index_fetchflags(mailbox, msgno, SYSTEM_FLAGS(msgno), user_flags,
+ LAST_UPDATED(msgno));
+ if (usinguid) prot_printf(imapd_out, " UID %u", UID(msgno));
+ prot_printf(imapd_out, ")\r\n");
+ }
+ }
+}
+
/*
* Checkpoint the user's \Seen state
*
More information about the Cyrus-devel
mailing list