commit 1631361f66162f0a85bdc3543bf72406b93d0cb1
Author: Oswald Buddenhagen <o...@users.sf.net>
Date:   Sat Apr 23 14:45:44 2022 +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.

 src/driver.h      |  3 ++-
 src/drv_imap.c    | 34 ++++++++++++++++++++++++++++------
 src/drv_maildir.c |  8 ++++----
 src/sync.c        | 25 +++++++++++++++++--------
 src/sync_p.h      |  2 +-
 5 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/src/driver.h b/src/driver.h
index 4ac79093..7abb5946 100644
--- 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
index 3c54a2d1..b58828ce 100644
--- 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
index 3bc730cc..d87ba3a9 100644
--- 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
index ab076708..b8518679 100644
--- 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
index 80dea84e..ada2084a 100644
--- 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


_______________________________________________
isync-devel mailing list
isync-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/isync-devel

Reply via email to