Browse Source

Revert "actually implement imap_commit_cmds()"

the CHECK command doesn't do what i thought; the formulation in the
specs was ambiguous - it really checks for new mail, rather than
committing, and each operation is supposed to be atomic. inefficient,
but safe. IMAP4rev2 eliminates the command altogether, subsuming its
function under NOOP.

consequently, the commit callback doesn't make sense for imap.
in principle, we could use it to coalesce multiple STOREs to counter the
inefficiency, but that won't happen any time soon, and the
implementation would look rather differently anyway.

as a "side effect", this fixes an assertion failure in imap_close_box()
when all flag sets failed (e.g., b/c the box was read-only), as their
callbacks would be short-cut in front of the completion of the CHECK
command, which was not sequenced before the close_box() call.

This reverts commit cfaa4848dd.
wip/socket-debug
Oswald Buddenhagen 5 months ago
parent
commit
8b8313997c
  1. 53
      src/drv_imap.c

53
src/drv_imap.c

@ -163,8 +163,7 @@ union imap_store {
// Command queue // Command queue
imap_cmd_t *pending, **pending_append; imap_cmd_t *pending, **pending_append;
imap_cmd_t *in_progress, **in_progress_append; imap_cmd_t *in_progress, **in_progress_append;
imap_cmd_t *wait_check, **wait_check_append; int nexttag, num_in_progress;
int nexttag, num_in_progress, num_wait_check;
uint buffer_mem; // Memory currently occupied by buffers in the queue uint buffer_mem; // Memory currently occupied by buffers in the queue
// Used during sequential operations like connect // Used during sequential operations like connect
@ -204,7 +203,6 @@ union imap_store {
uint data_len; \ uint data_len; \
uint uid; /* to identify fetch responses */ \ uint uid; /* to identify fetch responses */ \
char high_prio; /* if command is queued, put it at the front of the queue. */ \ char high_prio; /* if command is queued, put it at the front of the queue. */ \
char wait_check; /* Don't report success until subsequent CHECK success. */ \
char to_trash; /* we are storing to trash, not current. */ \ char to_trash; /* we are storing to trash, not current. */ \
char create; /* create the mailbox if we get an error which suggests so. */ \ char create; /* create the mailbox if we get an error which suggests so. */ \
char failok; /* Don't complain about NO (@1) / BAD (@2) response. */ \ char failok; /* Don't complain about NO (@1) / BAD (@2) response. */ \
@ -352,8 +350,6 @@ new_imap_cmd( uint size )
static void static void
done_imap_cmd( imap_store_t *ctx, imap_cmd_t *cmd, int response ) done_imap_cmd( imap_store_t *ctx, imap_cmd_t *cmd, int response )
{ {
if (cmd->param.wait_check)
ctx->num_wait_check--;
if (cmd->param.data) { if (cmd->param.data) {
free( cmd->param.data ); free( cmd->param.data );
cmd->param.data = NULL; cmd->param.data = NULL;
@ -482,18 +478,6 @@ flush_imap_cmds( imap_store_t *ctx )
} }
} }
static void
finalize_checked_imap_cmds( imap_store_t *ctx, int resp )
{
imap_cmd_t *cmd;
while ((cmd = ctx->wait_check)) {
if (!(ctx->wait_check = cmd->next))
ctx->wait_check_append = &ctx->wait_check;
done_imap_cmd( ctx, cmd, resp );
}
}
static void static void
cancel_pending_imap_cmds( imap_store_t *ctx ) cancel_pending_imap_cmds( imap_store_t *ctx )
{ {
@ -527,8 +511,6 @@ submit_imap_cmd( imap_store_t *ctx, imap_cmd_t *cmd )
assert( cmd ); assert( cmd );
assert( cmd->param.done ); assert( cmd->param.done );
if (cmd->param.wait_check)
ctx->num_wait_check++;
if ((ctx->pending && !cmd->param.high_prio) || !cmd_sendable( ctx, cmd )) { if ((ctx->pending && !cmd->param.high_prio) || !cmd_sendable( ctx, cmd )) {
if (ctx->pending && cmd->param.high_prio) { if (ctx->pending && cmd->param.high_prio) {
cmd->next = ctx->pending; cmd->next = ctx->pending;
@ -1943,13 +1925,7 @@ imap_socket_read( void *aux )
imap_ref( ctx ); imap_ref( ctx );
if (resp == RESP_CANCEL) if (resp == RESP_CANCEL)
imap_invoke_bad_callback( ctx ); imap_invoke_bad_callback( ctx );
if (resp == RESP_OK && cmdp->param.wait_check) {
cmdp->next = NULL;
*ctx->wait_check_append = cmdp;
ctx->wait_check_append = &cmdp->next;
} else {
done_imap_cmd( ctx, cmdp, resp ); done_imap_cmd( ctx, cmdp, resp );
}
if (imap_deref( ctx )) if (imap_deref( ctx ))
return; return;
if (ctx->canceling && !ctx->in_progress) { if (ctx->canceling && !ctx->in_progress) {
@ -1972,7 +1948,6 @@ get_cmd_result_p2( imap_store_t *ctx, imap_cmd_t *cmd, int response )
if (response != RESP_OK) { if (response != RESP_OK) {
done_imap_cmd( ctx, ocmd, response ); done_imap_cmd( ctx, ocmd, response );
} else { } else {
assert( !ocmd->param.wait_check );
ctx->uidnext = 1; ctx->uidnext = 1;
if (ocmd->param.to_trash) if (ocmd->param.to_trash)
ctx->trashnc = TrashKnown; ctx->trashnc = TrashKnown;
@ -1993,7 +1968,6 @@ imap_cancel_store( store_t *gctx )
sasl_dispose( &ctx->sasl ); sasl_dispose( &ctx->sasl );
#endif #endif
socket_close( &ctx->conn ); socket_close( &ctx->conn );
finalize_checked_imap_cmds( ctx, RESP_CANCEL );
cancel_sent_imap_cmds( ctx ); cancel_sent_imap_cmds( ctx );
cancel_pending_imap_cmds( ctx ); cancel_pending_imap_cmds( ctx );
free( ctx->ns_prefix ); free( ctx->ns_prefix );
@ -2053,7 +2027,7 @@ imap_free_store( store_t *gctx )
{ {
imap_store_t *ctx = (imap_store_t *)gctx; imap_store_t *ctx = (imap_store_t *)gctx;
assert( !ctx->pending && !ctx->in_progress && !ctx->wait_check ); assert( !ctx->pending && !ctx->in_progress );
if (ctx->state == SST_BAD) { if (ctx->state == SST_BAD) {
imap_cancel_store( gctx ); imap_cancel_store( gctx );
@ -2164,7 +2138,6 @@ imap_alloc_store( store_conf_t *conf, const char *label )
imap_socket_read, (void (*)(void *))flush_imap_cmds, ctx ); imap_socket_read, (void (*)(void *))flush_imap_cmds, ctx );
ctx->in_progress_append = &ctx->in_progress; ctx->in_progress_append = &ctx->in_progress;
ctx->pending_append = &ctx->pending; ctx->pending_append = &ctx->pending;
ctx->wait_check_append = &ctx->wait_check;
gotsrv: gotsrv:
ctx->conf = cfg; ctx->conf = cfg;
@ -2878,7 +2851,7 @@ imap_select_box( store_t *gctx, const char *name )
{ {
imap_store_t *ctx = (imap_store_t *)gctx; imap_store_t *ctx = (imap_store_t *)gctx;
assert( !ctx->pending && !ctx->in_progress && !ctx->wait_check ); assert( !ctx->pending && !ctx->in_progress );
reset_imap_messages( &ctx->msgs ); reset_imap_messages( &ctx->msgs );
@ -3278,9 +3251,7 @@ imap_flags_helper( imap_store_t *ctx, uint uid, char what, int flags,
char buf[256]; char buf[256];
buf[imap_make_flags( flags, buf )] = 0; buf[imap_make_flags( flags, buf )] = 0;
imap_cmd_t *cmd = imap_refcounted_new_cmd( &sts->gen ); imap_exec( ctx, imap_refcounted_new_cmd( &sts->gen ), imap_set_flags_p2,
cmd->param.wait_check = 1;
imap_exec( ctx, cmd, imap_set_flags_p2,
"UID STORE %u %cFLAGS.SILENT %s", uid, what, buf ); "UID STORE %u %cFLAGS.SILENT %s", uid, what, buf );
} }
@ -3352,7 +3323,7 @@ imap_close_box( store_t *gctx,
{ {
imap_store_t *ctx = (imap_store_t *)gctx; imap_store_t *ctx = (imap_store_t *)gctx;
assert( !ctx->num_wait_check ); assert( !ctx->pending && !ctx->in_progress );
if (ctx->opts & OPEN_UID_EXPUNGE) { if (ctx->opts & OPEN_UID_EXPUNGE) {
INIT_REFCOUNTED_STATE(imap_expunge_state_t, sts, cb, aux) INIT_REFCOUNTED_STATE(imap_expunge_state_t, sts, cb, aux)
@ -3672,7 +3643,6 @@ imap_cancel_cmds( store_t *gctx,
{ {
imap_store_t *ctx = (imap_store_t *)gctx; imap_store_t *ctx = (imap_store_t *)gctx;
finalize_checked_imap_cmds( ctx, RESP_CANCEL );
cancel_pending_imap_cmds( ctx ); cancel_pending_imap_cmds( ctx );
if (ctx->in_progress) { if (ctx->in_progress) {
ctx->canceling = 1; ctx->canceling = 1;
@ -3685,21 +3655,10 @@ imap_cancel_cmds( store_t *gctx,
/******************* imap_commit_cmds *******************/ /******************* imap_commit_cmds *******************/
static void imap_commit_cmds_p2( imap_store_t *, imap_cmd_t *, int );
static void static void
imap_commit_cmds( store_t *gctx ) imap_commit_cmds( store_t *gctx )
{ {
imap_store_t *ctx = (imap_store_t *)gctx; (void)gctx;
if (ctx->num_wait_check)
imap_exec( ctx, NULL, imap_commit_cmds_p2, "CHECK" );
}
static void
imap_commit_cmds_p2( imap_store_t *ctx, imap_cmd_t *cmd ATTR_UNUSED, int response )
{
finalize_checked_imap_cmds( ctx, response );
} }
/******************* imap_get_memory_usage *******************/ /******************* imap_get_memory_usage *******************/

Loading…
Cancel
Save