commit 1631361f66162f0a85bdc3543bf72406b93d0cb1
parent 1a1ac25bc867da5b934ddf8acf00f994f6033f25
Author: Oswald Buddenhagen <ossi@users.sf.net>
Date: Sat, 23 Apr 2022 14:45:44 +0200
revamp handling of expunged messages
try to purge sync entries based on which messages are *actually*
expunged, rather than those that are *expected* to be expunged.
to save network bandwidth, the IMAP driver doesn't report all expunges,
so some entry purges would be delayed - potentially indefinitely, e.g.,
when only --pull-new --push is used, or Trash isn't used (nor
ExpungeSolo, prospectively). so keep a fallback path to avoid this.
Diffstat:
5 files changed, 52 insertions(+), 20 deletions(-)
diff --git a/src/driver.h b/src/driver.h
@@ -283,8 +283,9 @@ struct driver {
/* Expunge deleted messages from the current mailbox and close it.
* There is no need to explicitly close a mailbox if no expunge is needed. */
+ // If reported is true, the expunge callback was called reliably.
void (*close_box)( store_t *ctx,
- void (*cb)( int sts, void *aux ), void *aux );
+ void (*cb)( int sts, int reported, void *aux ), void *aux );
/* Cancel queued commands which are not in flight yet; they will have their
* callbacks invoked with DRV_CANCELED. Afterwards, wait for the completion of
diff --git a/src/drv_imap.c b/src/drv_imap.c
@@ -3082,21 +3082,33 @@ imap_set_flags_p3( imap_set_msg_flags_state_t *sts )
/******************* imap_close_box *******************/
+#define IMAP_CMD_EXPUNGE \
+ void (*callback)( int sts, int reported, void *aux ); \
+ void *callback_aux;
+
typedef union {
imap_cmd_refcounted_state_t gen;
struct {
IMAP_CMD_REFCOUNTED_STATE
- void (*callback)( int sts, void *aux );
- void *callback_aux;
+ IMAP_CMD_EXPUNGE
};
} imap_expunge_state_t;
+typedef union {
+ imap_cmd_t gen;
+ struct {
+ IMAP_CMD
+ IMAP_CMD_EXPUNGE
+ };
+} imap_cmd_close_t;
+
static void imap_close_box_p2( imap_store_t *, imap_cmd_t *, int );
static void imap_close_box_p3( imap_expunge_state_t * );
+static void imap_close_box_simple_p2( imap_store_t *, imap_cmd_t *, int );
static void
imap_close_box( store_t *gctx,
- void (*cb)( int sts, void *aux ), void *aux )
+ void (*cb)( int sts, int reported, void *aux ), void *aux )
{
imap_store_t *ctx = (imap_store_t *)gctx;
@@ -3132,8 +3144,8 @@ imap_close_box( store_t *gctx,
// Note that, to save bandwidth, we don't use EXPUNGE. Also, in many
// cases, we wouldn't be able to map the EXPUNGE responses' seq numbers
// anyway, due to not having fetched the messages.
- INIT_IMAP_CMD(imap_cmd_simple_t, cmd, cb, aux)
- imap_exec( ctx, &cmd->gen, imap_done_simple_box, "CLOSE" );
+ INIT_IMAP_CMD(imap_cmd_close_t, cmd, cb, aux)
+ imap_exec( ctx, &cmd->gen, imap_close_box_simple_p2, "CLOSE" );
}
}
@@ -3149,7 +3161,17 @@ imap_close_box_p2( imap_store_t *ctx ATTR_UNUSED, imap_cmd_t *cmd, int response
static void
imap_close_box_p3( imap_expunge_state_t *sts )
{
- DONE_REFCOUNTED_STATE(sts)
+ DONE_REFCOUNTED_STATE_ARGS(sts, , 1)
+}
+
+static void
+imap_close_box_simple_p2( imap_store_t *ctx ATTR_UNUSED,
+ imap_cmd_t *cmd, int response )
+{
+ imap_cmd_close_t *cmdp = (imap_cmd_close_t *)cmd;
+
+ transform_box_response( &response );
+ cmdp->callback( response, 0, cmdp->callback_aux );
}
/******************* imap_trash_msg *******************/
diff --git a/src/drv_maildir.c b/src/drv_maildir.c
@@ -1795,7 +1795,7 @@ maildir_trash_msg( store_t *gctx, message_t *gmsg,
static void
maildir_close_box( store_t *gctx,
- void (*cb)( int sts, void *aux ), void *aux )
+ void (*cb)( int sts, int reported, void *aux ), void *aux )
{
maildir_store_t *ctx = (maildir_store_t *)gctx;
maildir_message_t *msg;
@@ -1819,7 +1819,7 @@ maildir_close_box( store_t *gctx,
ctx->expunge_callback( &msg->gen, ctx->callback_aux );
#ifdef USE_DB
if (ctx->db && (ret = maildir_purge_msg( ctx, msg->base )) != DRV_OK) {
- cb( ret, aux );
+ cb( ret, 1, aux );
return;
}
#endif /* USE_DB */
@@ -1827,11 +1827,11 @@ maildir_close_box( store_t *gctx,
}
}
if (!retry) {
- cb( DRV_OK, aux );
+ cb( DRV_OK, 1, aux );
return;
}
if ((ret = maildir_rescan( ctx )) != DRV_OK) {
- cb( ret, aux );
+ cb( ret, 1, aux );
return;
}
}
diff --git a/src/sync.c b/src/sync.c
@@ -58,7 +58,6 @@ BIT_ENUM(
ST_SENT_TRASH,
ST_CLOSING,
ST_CLOSED,
- ST_DID_EXPUNGE,
ST_SENT_CANCEL,
ST_CANCELED,
)
@@ -465,6 +464,7 @@ message_expunged( message_t *msg, void *aux )
(void)svars;
if (msg->srec) {
+ msg->srec->status |= S_GONE(t);
msg->srec->msg[t] = NULL;
msg->srec = NULL;
}
@@ -1867,7 +1867,7 @@ msg_rtrashed( int sts, uint uid ATTR_UNUSED, copy_vars_t *vars )
sync_close( svars, t );
}
-static void box_closed( int sts, void *aux );
+static void box_closed( int sts, int reported, void *aux );
static void box_closed_p2( sync_vars_t *svars, int t );
static void
@@ -1891,10 +1891,23 @@ sync_close( sync_vars_t *svars, int t )
}
static void
-box_closed( int sts, void *aux )
+box_closed( int sts, int reported, void *aux )
{
SVARS_CHECK_RET;
- svars->state[t] |= ST_DID_EXPUNGE;
+ if (!reported) {
+ for (sync_rec_t *srec = svars->srecs; srec; srec = srec->next) {
+ if (srec->status & S_DEAD)
+ continue;
+ // Note that this logic is somewhat optimistic - theoretically, it's
+ // possible that a message was concurrently undeleted before we tried
+ // to expunge it. Such a message would be subsequently re-propagated
+ // by a refresh, and in the extremely unlikely case of this happening
+ // on both sides, we'd even get a duplicate. That's why this is only
+ // a fallback.
+ if (srec->status & S_DEL(t))
+ srec->status |= S_GONE(t);
+ }
+ }
box_closed_p2( svars, t );
}
@@ -1931,10 +1944,6 @@ box_closed_p2( sync_vars_t *svars, int t )
for (srec = svars->srecs; srec; srec = srec->next) {
if (srec->status & S_DEAD)
continue;
- if ((srec->status & S_DEL(F)) && (svars->state[F] & ST_DID_EXPUNGE))
- srec->status |= S_GONE(F);
- if ((srec->status & S_DEL(N)) && (svars->state[N] & ST_DID_EXPUNGE))
- srec->status |= S_GONE(N);
if (!srec->uid[N] || (srec->status & S_GONE(N))) {
if (!srec->uid[F] || (srec->status & S_GONE(F)) ||
((srec->status & S_EXPIRED) && svars->maxuid[F] >= srec->uid[F] && svars->maxxfuid >= srec->uid[F])) {
diff --git a/src/sync_p.h b/src/sync_p.h
@@ -18,7 +18,7 @@ BIT_ENUM(
S_DUMMY(2), // f/n message is only a placeholder
S_SKIPPED, // pre-1.4 legacy: the entry was not propagated (message is too big)
S_GONE(2), // ephemeral: f/n message has been expunged
- S_DEL(2), // ephemeral: f/n message would be subject to expunge
+ S_DEL(2), // ephemeral: f/n message would be subject to non-selective expunge
S_DELETE, // ephemeral: flags propagation is a deletion
S_UPGRADE, // ephemeral: upgrading placeholder, do not apply MaxSize
S_PURGE, // ephemeral: placeholder is being nuked
