fix possible attempts to set flags of M_DEAD messages
so far, we checked for M_DEAD only in loops over messages. but we should have checked srec->msg uses as well. this would make the code a mess, so instead call back from the drivers when messages are expunged, so we can reset the pointers. the only case where this really matters so far is the flag setting loop, which may cause the concurrent expunge of not yet handled messages to be detected by the maildir driver.
This commit is contained in:
parent
87d1a4edde
commit
4e25fd59c1
11
src/driver.h
11
src/driver.h
|
@ -166,9 +166,14 @@ struct driver {
|
|||
* return quickly, and must not fail. */
|
||||
store_t *(*alloc_store)( store_conf_t *conf, const char *label );
|
||||
|
||||
/* When this callback is invoked (at most once per store), the store is fubar;
|
||||
* call cancel_store() to dispose of it. */
|
||||
void (*set_bad_callback)( store_t *ctx, void (*cb)( void *aux ), void *aux );
|
||||
// When exp_cb is invoked, the passed message has been expunged;
|
||||
// its status is M_DEAD now.
|
||||
// When bad_cb is invoked (at most once per store), the store is fubar;
|
||||
// call cancel_store() to dispose of it.
|
||||
void (*set_callbacks)( store_t *ctx,
|
||||
void (*exp_cb)( message_t *msg, void *aux ),
|
||||
void (*bad_cb)( void *aux ),
|
||||
void *aux );
|
||||
|
||||
/* Open/connect the store. This may recycle existing server connections. */
|
||||
void (*connect_store)( store_t *ctx,
|
||||
|
|
|
@ -1787,7 +1787,8 @@ imap_deref( imap_store_t *ctx )
|
|||
}
|
||||
|
||||
static void
|
||||
imap_set_bad_callback( store_t *gctx, void (*cb)( void *aux ), void *aux )
|
||||
imap_set_callbacks( store_t *gctx, void (*exp_cb)( message_t *, void * ) ATTR_UNUSED,
|
||||
void (*cb)( void * ), void *aux )
|
||||
{
|
||||
imap_store_t *ctx = (imap_store_t *)gctx;
|
||||
|
||||
|
@ -1833,7 +1834,7 @@ imap_free_store( store_t *gctx )
|
|||
|
||||
free_generic_messages( &ctx->msgs->gen );
|
||||
ctx->msgs = NULL;
|
||||
imap_set_bad_callback( gctx, imap_cancel_unowned, gctx );
|
||||
imap_set_callbacks( gctx, NULL, imap_cancel_unowned, gctx );
|
||||
ctx->next = unowned;
|
||||
unowned = ctx;
|
||||
}
|
||||
|
@ -1849,7 +1850,7 @@ imap_cleanup( void )
|
|||
|
||||
for (ctx = unowned; ctx; ctx = nctx) {
|
||||
nctx = ctx->next;
|
||||
imap_set_bad_callback( &ctx->gen, (void (*)(void *))imap_cancel_store, ctx );
|
||||
imap_set_callbacks( &ctx->gen, NULL, (void (*)( void * ))imap_cancel_store, ctx );
|
||||
ctx->expectBYE = 1;
|
||||
imap_exec( ctx, NULL, imap_cleanup_p2, "LOGOUT" );
|
||||
}
|
||||
|
@ -3795,7 +3796,7 @@ struct driver imap_driver = {
|
|||
imap_parse_store,
|
||||
imap_cleanup,
|
||||
imap_alloc_store,
|
||||
imap_set_bad_callback,
|
||||
imap_set_callbacks,
|
||||
imap_connect_store,
|
||||
imap_free_store,
|
||||
imap_cancel_store,
|
||||
|
|
|
@ -75,8 +75,9 @@ typedef union maildir_store {
|
|||
maildir_message_t *msgs;
|
||||
wakeup_t lcktmr;
|
||||
|
||||
void (*expunge_callback)( message_t *msg, void *aux );
|
||||
void (*bad_callback)( void *aux );
|
||||
void *bad_callback_aux;
|
||||
void *callback_aux;
|
||||
};
|
||||
} maildir_store_t;
|
||||
|
||||
|
@ -277,18 +278,20 @@ maildir_cleanup_drv( void )
|
|||
}
|
||||
|
||||
static void
|
||||
maildir_set_bad_callback( store_t *gctx, void (*cb)( void *aux ), void *aux )
|
||||
maildir_set_callbacks( store_t *gctx, void (*exp_cb)( message_t *, void * ),
|
||||
void (*bad_cb)( void * ), void *aux )
|
||||
{
|
||||
maildir_store_t *ctx = (maildir_store_t *)gctx;
|
||||
|
||||
ctx->bad_callback = cb;
|
||||
ctx->bad_callback_aux = aux;
|
||||
ctx->expunge_callback = exp_cb;
|
||||
ctx->bad_callback = bad_cb;
|
||||
ctx->callback_aux = aux;
|
||||
}
|
||||
|
||||
static void
|
||||
maildir_invoke_bad_callback( maildir_store_t *ctx )
|
||||
{
|
||||
ctx->bad_callback( ctx->bad_callback_aux );
|
||||
ctx->bad_callback( ctx->callback_aux );
|
||||
}
|
||||
|
||||
static int
|
||||
|
@ -1457,6 +1460,7 @@ maildir_rescan( maildir_store_t *ctx )
|
|||
} else if (i >= msglist.array.size) {
|
||||
debug( " purging deleted message %u\n", msg->uid );
|
||||
msg->status = M_DEAD;
|
||||
ctx->expunge_callback( &msg->gen, ctx->callback_aux );
|
||||
msgapp = &msg->next;
|
||||
} else if (msglist.array.data[i].uid < msg->uid) {
|
||||
/* this should not happen, actually */
|
||||
|
@ -1470,6 +1474,7 @@ maildir_rescan( maildir_store_t *ctx )
|
|||
} else if (msglist.array.data[i].uid > msg->uid) {
|
||||
debug( " purging deleted message %u\n", msg->uid );
|
||||
msg->status = M_DEAD;
|
||||
ctx->expunge_callback( &msg->gen, ctx->callback_aux );
|
||||
msgapp = &msg->next;
|
||||
} else {
|
||||
debug( " updating message %u\n", msg->uid );
|
||||
|
@ -1765,6 +1770,7 @@ maildir_trash_msg( store_t *gctx, message_t *gmsg,
|
|||
}
|
||||
gmsg->status |= M_DEAD;
|
||||
ctx->total_msgs--;
|
||||
ctx->expunge_callback( gmsg, ctx->callback_aux );
|
||||
|
||||
#ifdef USE_DB
|
||||
if (ctx->usedb) {
|
||||
|
@ -1798,6 +1804,7 @@ maildir_close_box( store_t *gctx,
|
|||
} else {
|
||||
msg->status |= M_DEAD;
|
||||
ctx->total_msgs--;
|
||||
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 );
|
||||
|
@ -1914,7 +1921,7 @@ struct driver maildir_driver = {
|
|||
maildir_parse_store,
|
||||
maildir_cleanup_drv,
|
||||
maildir_alloc_store,
|
||||
maildir_set_bad_callback,
|
||||
maildir_set_callbacks,
|
||||
maildir_connect_store,
|
||||
maildir_free_store,
|
||||
maildir_free_store, /* _cancel_, but it's the same */
|
||||
|
|
|
@ -25,8 +25,9 @@ typedef union proxy_store {
|
|||
wakeup_t wakeup;
|
||||
char force_async;
|
||||
|
||||
void (*expunge_callback)( message_t *msg, void *aux );
|
||||
void (*bad_callback)( void *aux );
|
||||
void *bad_callback_aux;
|
||||
void *callback_aux;
|
||||
};
|
||||
} proxy_store_t;
|
||||
|
||||
|
@ -337,14 +338,26 @@ static @type@proxy_@name@( store_t *gctx@decl_args@, void (*cb)( @decl_cb_args@v
|
|||
//# END
|
||||
#endif
|
||||
|
||||
//# SPECIAL set_bad_callback
|
||||
//# SPECIAL set_callbacks
|
||||
static void
|
||||
proxy_set_bad_callback( store_t *gctx, void (*cb)( void *aux ), void *aux )
|
||||
proxy_set_callbacks( store_t *gctx, void (*exp_cb)( message_t *, void * ),
|
||||
void (*bad_cb)( void * ), void *aux )
|
||||
{
|
||||
proxy_store_t *ctx = (proxy_store_t *)gctx;
|
||||
|
||||
ctx->bad_callback = cb;
|
||||
ctx->bad_callback_aux = aux;
|
||||
ctx->expunge_callback = exp_cb;
|
||||
ctx->bad_callback = bad_cb;
|
||||
ctx->callback_aux = aux;
|
||||
}
|
||||
|
||||
static void
|
||||
proxy_invoke_expunge_callback( message_t *msg, proxy_store_t *ctx )
|
||||
{
|
||||
ctx->ref_count++;
|
||||
debug( "%sCallback enter expunged message %u\n", ctx->label, msg->uid );
|
||||
ctx->expunge_callback( msg, ctx->callback_aux );
|
||||
debug( "%sCallback leave expunged message %u\n", ctx->label, msg->uid );
|
||||
proxy_store_deref( ctx );
|
||||
}
|
||||
|
||||
static void
|
||||
|
@ -352,7 +365,7 @@ proxy_invoke_bad_callback( proxy_store_t *ctx )
|
|||
{
|
||||
ctx->ref_count++;
|
||||
debug( "%sCallback enter bad store\n", ctx->label );
|
||||
ctx->bad_callback( ctx->bad_callback_aux );
|
||||
ctx->bad_callback( ctx->callback_aux );
|
||||
debug( "%sCallback leave bad store\n", ctx->label );
|
||||
proxy_store_deref( ctx );
|
||||
}
|
||||
|
@ -373,7 +386,9 @@ proxy_alloc_store( store_t *real_ctx, const char *label, int force_async )
|
|||
ctx->check_cmds_append = &ctx->check_cmds;
|
||||
ctx->real_driver = real_ctx->driver;
|
||||
ctx->real_store = real_ctx;
|
||||
ctx->real_driver->set_bad_callback( ctx->real_store, (void (*)(void *))proxy_invoke_bad_callback, ctx );
|
||||
ctx->real_driver->set_callbacks( ctx->real_store,
|
||||
(void (*)( message_t *, void * ))proxy_invoke_expunge_callback,
|
||||
(void (*)( void * ))proxy_invoke_bad_callback, ctx );
|
||||
init_wakeup( &ctx->wakeup, proxy_wakeup, ctx );
|
||||
return &ctx->gen;
|
||||
}
|
||||
|
|
|
@ -114,7 +114,7 @@ do_list_stores( list_vars_t *lvars )
|
|||
ctx = proxy_alloc_store( ctx, "", DFlags & FORCEASYNC(F) );
|
||||
}
|
||||
lvars->ctx = ctx;
|
||||
lvars->drv->set_bad_callback( ctx, list_store_bad, lvars );
|
||||
lvars->drv->set_callbacks( ctx, NULL, list_store_bad, lvars );
|
||||
info( "Opening store %s...\n", lvars->store->name );
|
||||
lvars->cben = lvars->done = 0;
|
||||
lvars->drv->connect_store( lvars->ctx, list_store_connected, lvars );
|
||||
|
|
|
@ -412,7 +412,7 @@ do_sync_chans( main_vars_t *mvars )
|
|||
ctx = proxy_alloc_store( ctx, labels[t], DFlags & FORCEASYNC(t) );
|
||||
}
|
||||
mvars->ctx[t] = ctx;
|
||||
mvars->drv[t]->set_bad_callback( ctx, store_bad, AUX );
|
||||
mvars->drv[t]->set_callbacks( ctx, NULL, store_bad, AUX );
|
||||
mvars->state[t] = ST_FRESH;
|
||||
}
|
||||
mvars->chan_cben = 0;
|
||||
|
|
26
src/sync.c
26
src/sync.c
|
@ -432,6 +432,18 @@ check_ret( int sts, void *aux )
|
|||
} \
|
||||
INIT_SVARS(vars->aux)
|
||||
|
||||
static void
|
||||
message_expunged( message_t *msg, void *aux )
|
||||
{
|
||||
DECL_INIT_SVARS(aux);
|
||||
(void)svars;
|
||||
|
||||
if (msg->srec) {
|
||||
msg->srec->msg[t] = NULL;
|
||||
msg->srec = NULL;
|
||||
}
|
||||
}
|
||||
|
||||
static void box_confirmed( int sts, uint uidvalidity, void *aux );
|
||||
static void box_confirmed2( sync_vars_t *svars, int t );
|
||||
static void box_deleted( int sts, void *aux );
|
||||
|
@ -473,7 +485,7 @@ sync_boxes( store_t *ctx[], const char * const names[], int present[], channel_c
|
|||
return;
|
||||
}
|
||||
svars->drv[t] = ctx[t]->driver;
|
||||
svars->drv[t]->set_bad_callback( ctx[t], store_bad, AUX );
|
||||
svars->drv[t]->set_callbacks( ctx[t], message_expunged, store_bad, AUX );
|
||||
svars->can_crlf[t] = (svars->drv[t]->get_caps( svars->ctx[t] ) / DRV_CRLF) & 1;
|
||||
}
|
||||
/* Both boxes must be fully set up at this point, so that error exit paths
|
||||
|
@ -1259,6 +1271,13 @@ box_loaded( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux
|
|||
for (t = 0; t < 2; t++) {
|
||||
if (!srec->uid[t])
|
||||
continue;
|
||||
if (!srec->msg[t] && (svars->opts[t] & OPEN_OLD)) {
|
||||
// The message was expunged. No need to call flags_set(), because:
|
||||
// - for S_DELETE and S_PURGE, the entry will be pruned due to both sides being gone
|
||||
// - for regular flag propagations, there is nothing to do
|
||||
// - expirations were already handled above
|
||||
continue;
|
||||
}
|
||||
aflags = srec->aflags[t];
|
||||
dflags = srec->dflags[t];
|
||||
if (srec->status & (S_DELETE | S_PURGE)) {
|
||||
|
@ -1267,11 +1286,6 @@ box_loaded( int sts, message_t *msgs, int total_msgs, int recent_msgs, void *aux
|
|||
// this deletion of a dummy happens on the other side.
|
||||
continue;
|
||||
}
|
||||
if (!srec->msg[t] && (svars->opts[t] & OPEN_OLD)) {
|
||||
// The message disappeared. This can happen, because the status may
|
||||
// come from the journal, and things could have happened meanwhile.
|
||||
continue;
|
||||
}
|
||||
} else {
|
||||
/* The trigger is an expiration transaction being ongoing ... */
|
||||
if ((t == N) && ((shifted_bit(srec->status, S_EXPIRE, S_EXPIRED) ^ srec->status) & S_EXPIRED)) {
|
||||
|
|
Loading…
Reference in New Issue
Block a user