From 9d22641b62fdfb6c9eb1a47f49db444890df5e84 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Sun, 24 May 2015 11:37:15 +0200 Subject: [PATCH] make server connection a cancellable operation this entails splitting drv->open_store() into alloc_store() and connect_store(). --- src/driver.h | 21 ++++--- src/drv_imap.c | 108 ++++++++++++++++++--------------- src/drv_maildir.c | 42 +++++++------ src/main.c | 150 ++++++++++++++++++++++++++-------------------- src/socket.c | 8 +++ 5 files changed, 190 insertions(+), 139 deletions(-) diff --git a/src/driver.h b/src/driver.h index dcf9255..358f52d 100644 --- a/src/driver.h +++ b/src/driver.h @@ -121,8 +121,10 @@ typedef struct { #define DRV_MSG_BAD 1 /* Something is wrong with the current mailbox - probably it is somehow inaccessible. */ #define DRV_BOX_BAD 2 +/* Failed to connect store. */ +#define DRV_STORE_BAD 3 /* The command has been cancel()ed or cancel_store()d. */ -#define DRV_CANCELED 3 +#define DRV_CANCELED 4 /* All memory belongs to the driver's user, unless stated otherwise. */ @@ -146,16 +148,19 @@ struct driver { /* Parse configuration. */ int (*parse_store)( conffile_t *cfg, store_conf_t **storep ); - /* Close remaining server connections. All stores must be disowned first. */ + /* Close remaining server connections. All stores must be discarded first. */ void (*cleanup)( void ); - /* Open a store with the given configuration. This may recycle existing - * server connections. Upon failure, a null store is passed to the callback. */ - void (*open_store)( store_conf_t *conf, const char *label, - void (*cb)( store_t *ctx, void *aux ), void *aux ); + /* Allocate a store with the given configuration. This is expected to + * return quickly, and must not fail. */ + store_t *(*alloc_store)( store_conf_t *conf, const char *label ); - /* Mark the store as available for recycling. Server connection may be kept alive. */ - void (*disown_store)( store_t *ctx ); + /* Open/connect the store. This may recycle existing server connections. */ + void (*connect_store)( store_t *ctx, + void (*cb)( int sts, void *aux ), void *aux ); + + /* Discard the store. Underlying server connection may be kept alive. */ + void (*free_store)( store_t *ctx ); /* Discard the store after a bad_callback. The server connections will be closed. * Pending commands will have their callbacks synchronously invoked with DRV_CANCELED. */ diff --git a/src/drv_imap.c b/src/drv_imap.c index a1f73d6..474125b 100644 --- a/src/drv_imap.c +++ b/src/drv_imap.c @@ -100,6 +100,7 @@ typedef struct imap_store { const char *prefix; const char *name; int ref_count; + enum { SST_BAD, SST_HALF, SST_GOOD } state; /* trash folder's existence is not confirmed yet */ enum { TrashUnknown, TrashChecking, TrashKnown } trashnc; uint got_namespace:1; @@ -121,7 +122,7 @@ typedef struct imap_store { int expectEOF; /* received LOGOUT's OK or unsolicited BYE */ int canceling; /* imap_cancel() is in progress */ union { - void (*imap_open)( store_t *srv, void *aux ); + void (*imap_open)( int sts, void *aux ); void (*imap_cancel)( void *aux ); } callbacks; void *callback_aux; @@ -1423,6 +1424,15 @@ get_cmd_result_p2( imap_store_t *ctx, struct imap_cmd *cmd, int response ) /******************* imap_cancel_store *******************/ + +static void +imap_cleanup_store( imap_store_t *ctx ) +{ + free_generic_messages( ctx->gen.msgs ); + free_string_list( ctx->gen.boxes ); + free( ctx->delimiter ); +} + static void imap_cancel_store( store_t *gctx ) { @@ -1434,13 +1444,11 @@ imap_cancel_store( store_t *gctx ) socket_close( &ctx->conn ); cancel_sent_imap_cmds( ctx ); cancel_pending_imap_cmds( ctx ); - free_generic_messages( ctx->gen.msgs ); - free_string_list( ctx->gen.boxes ); free_list( ctx->ns_personal ); free_list( ctx->ns_other ); free_list( ctx->ns_shared ); free_string_list( ctx->auth_mechs ); - free( ctx->delimiter ); + imap_cleanup_store( ctx ); imap_deref( ctx ); } @@ -1460,7 +1468,7 @@ imap_invoke_bad_callback( imap_store_t *ctx ) ctx->gen.bad_callback( ctx->gen.bad_callback_aux ); } -/******************* imap_disown_store *******************/ +/******************* imap_free_store *******************/ static store_t *unowned; @@ -1478,7 +1486,7 @@ imap_cancel_unowned( void *gctx ) } static void -imap_disown_store( store_t *gctx ) +imap_free_store( store_t *gctx ) { free_generic_messages( gctx->msgs ); gctx->msgs = 0; @@ -1542,61 +1550,64 @@ static void imap_open_store_ssl_bail( imap_store_t * ); #endif static void imap_open_store_bail( imap_store_t *, int ); -static void -imap_open_store_bad( void *aux ) -{ - imap_open_store_bail( (imap_store_t *)aux, FAIL_TEMP ); -} - -static void -imap_open_store( store_conf_t *conf, const char *label, - void (*cb)( store_t *srv, void *aux ), void *aux ) +static store_t * +imap_alloc_store( store_conf_t *conf, const char *label ) { imap_store_conf_t *cfg = (imap_store_conf_t *)conf; imap_server_conf_t *srvc = cfg->server; imap_store_t *ctx; store_t **ctxp; + /* First try to recycle a whole store. */ for (ctxp = &unowned; (ctx = (imap_store_t *)*ctxp); ctxp = &ctx->gen.next) - if (ctx->gen.conf == conf) { + if (ctx->state == SST_GOOD && ctx->gen.conf == conf) { *ctxp = ctx->gen.next; ctx->label = label; - cb( &ctx->gen, aux ); - return; + return &ctx->gen; } + + /* Then try to recycle a server connection. */ for (ctxp = &unowned; (ctx = (imap_store_t *)*ctxp); ctxp = &ctx->gen.next) - if (((imap_store_conf_t *)ctx->gen.conf)->server == srvc) { + if (ctx->state != SST_BAD && ((imap_store_conf_t *)ctx->gen.conf)->server == srvc) { *ctxp = ctx->gen.next; - ctx->label = label; + imap_cleanup_store( ctx ); /* One could ping the server here, but given that the idle timeout * is at least 30 minutes, this sounds pretty pointless. */ - free_string_list( ctx->gen.boxes ); - ctx->gen.boxes = 0; - ctx->gen.listed = 0; - ctx->gen.conf = conf; - free( ctx->delimiter ); - ctx->delimiter = 0; - ctx->callbacks.imap_open = cb; - ctx->callback_aux = aux; - set_bad_callback( &ctx->gen, imap_open_store_bad, ctx ); - imap_open_store_namespace( ctx ); - return; + ctx->state = SST_HALF; + goto gotsrv; } + /* Finally, schedule opening a new server connection. */ ctx = nfcalloc( sizeof(*ctx) ); - ctx->gen.conf = conf; - ctx->label = label; - ctx->ref_count = 1; - ctx->callbacks.imap_open = cb; - ctx->callback_aux = aux; - set_bad_callback( &ctx->gen, imap_open_store_bad, ctx ); - ctx->in_progress_append = &ctx->in_progress; - ctx->pending_append = &ctx->pending; - socket_init( &ctx->conn, &srvc->sconf, (void (*)( void * ))imap_invoke_bad_callback, imap_socket_read, (void (*)(void *))flush_imap_cmds, ctx ); - socket_connect( &ctx->conn, imap_open_store_connected ); + ctx->in_progress_append = &ctx->in_progress; + ctx->pending_append = &ctx->pending; + + gotsrv: + ctx->gen.conf = conf; + ctx->label = label; + ctx->ref_count = 1; + return &ctx->gen; +} + +static void +imap_connect_store( store_t *gctx, + void (*cb)( int sts, void *aux ), void *aux ) +{ + imap_store_t *ctx = (imap_store_t *)gctx; + + if (ctx->state == SST_GOOD) { + cb( DRV_OK, aux ); + } else { + ctx->callbacks.imap_open = cb; + ctx->callback_aux = aux; + if (ctx->state == SST_HALF) + imap_open_store_namespace( ctx ); + else + socket_connect( &ctx->conn, imap_open_store_connected ); + } } static void @@ -2091,6 +2102,7 @@ imap_open_store_namespace( imap_store_t *ctx ) { imap_store_conf_t *cfg = (imap_store_conf_t *)ctx->gen.conf; + ctx->state = SST_HALF; ctx->prefix = cfg->gen.path; ctx->delimiter = cfg->delimiter ? nfstrdup( cfg->delimiter ) : 0; if (((!ctx->prefix && cfg->use_namespace) || !cfg->delimiter) && CAP(NAMESPACE)) { @@ -2140,11 +2152,11 @@ imap_open_store_namespace2( imap_store_t *ctx ) static void imap_open_store_finalize( imap_store_t *ctx ) { - set_bad_callback( &ctx->gen, 0, 0 ); + ctx->state = SST_GOOD; if (!ctx->prefix) ctx->prefix = ""; ctx->trashnc = TrashUnknown; - ctx->callbacks.imap_open( &ctx->gen, ctx->callback_aux ); + ctx->callbacks.imap_open( DRV_OK, ctx->callback_aux ); } #ifdef HAVE_LIBSSL @@ -2160,11 +2172,8 @@ imap_open_store_ssl_bail( imap_store_t *ctx ) static void imap_open_store_bail( imap_store_t *ctx, int failed ) { - void (*cb)( store_t *srv, void *aux ) = ctx->callbacks.imap_open; - void *aux = ctx->callback_aux; ((imap_store_conf_t *)ctx->gen.conf)->server->failed = failed; - imap_cancel_store( &ctx->gen ); - cb( 0, aux ); + ctx->callbacks.imap_open( DRV_STORE_BAD, ctx->callback_aux ); } /******************* imap_open_box *******************/ @@ -2945,8 +2954,9 @@ struct driver imap_driver = { DRV_CRLF | DRV_VERBOSE, imap_parse_store, imap_cleanup, - imap_open_store, - imap_disown_store, + imap_alloc_store, + imap_connect_store, + imap_free_store, imap_cancel_store, imap_list_store, imap_select_box, diff --git a/src/drv_maildir.c b/src/drv_maildir.c index b336ba9..3f000ff 100644 --- a/src/drv_maildir.c +++ b/src/drv_maildir.c @@ -194,35 +194,40 @@ maildir_validate_path( maildir_store_conf_t *conf ) static void lcktmr_timeout( void *aux ); -static void -maildir_open_store( store_conf_t *gconf, const char *label ATTR_UNUSED, - void (*cb)( store_t *ctx, void *aux ), void *aux ) +static store_t * +maildir_alloc_store( store_conf_t *gconf, const char *label ATTR_UNUSED ) { - maildir_store_conf_t *conf = (maildir_store_conf_t *)gconf; maildir_store_t *ctx; ctx = nfcalloc( sizeof(*ctx) ); ctx->gen.conf = gconf; ctx->uvfd = -1; init_wakeup( &ctx->lcktmr, lcktmr_timeout, ctx ); - if (gconf->path && maildir_validate_path( conf ) < 0) { - free( ctx ); - cb( 0, aux ); + return &ctx->gen; +} + +static void +maildir_connect_store( store_t *gctx, + void (*cb)( int sts, void *aux ), void *aux ) +{ + maildir_store_t *ctx = (maildir_store_t *)gctx; + maildir_store_conf_t *conf = (maildir_store_conf_t *)ctx->gen.conf; + + if (conf->gen.path && maildir_validate_path( conf ) < 0) { + cb( DRV_STORE_BAD, aux ); return; } - if (gconf->trash) { + if (conf->gen.trash) { if (maildir_ensure_path( conf ) < 0) { - free( ctx ); - cb( 0, aux ); + cb( DRV_STORE_BAD, aux ); return; } - if (!(ctx->trash = maildir_join_path( conf, gconf->path, gconf->trash ))) { - free( ctx ); - cb( 0, aux ); + if (!(ctx->trash = maildir_join_path( conf, conf->gen.path, conf->gen.trash ))) { + cb( DRV_STORE_BAD, aux ); return; } } - cb( &ctx->gen, aux ); + cb( DRV_OK, aux ); } static void @@ -256,7 +261,7 @@ maildir_cleanup( store_t *gctx ) } static void -maildir_disown_store( store_t *gctx ) +maildir_free_store( store_t *gctx ) { maildir_store_t *ctx = (maildir_store_t *)gctx; @@ -1761,9 +1766,10 @@ struct driver maildir_driver = { 0, /* XXX DRV_CRLF? */ maildir_parse_store, maildir_cleanup_drv, - maildir_open_store, - maildir_disown_store, - maildir_disown_store, /* _cancel_, but it's the same */ + maildir_alloc_store, + maildir_connect_store, + maildir_free_store, + maildir_free_store, /* _cancel_, but it's the same */ maildir_list_store, maildir_select_box, maildir_create_box, diff --git a/src/main.c b/src/main.c index 85a6b06..fa0b936 100644 --- a/src/main.c +++ b/src/main.c @@ -727,10 +727,33 @@ main( int argc, char **argv ) } #define ST_FRESH 0 -#define ST_OPEN 1 -#define ST_CLOSED 2 +#define ST_CONNECTED 1 +#define ST_OPEN 2 +#define ST_CANCELING 3 +#define ST_CLOSED 4 -static void store_opened( store_t *ctx, void *aux ); +static void +cancel_prep_done( void *aux ) +{ + MVARS(aux) + + mvars->drv[t]->free_store( mvars->ctx[t] ); + mvars->state[t] = ST_CLOSED; + sync_chans( mvars, E_OPEN ); +} + +static void +store_bad( void *aux ) +{ + MVARS(aux) + + mvars->drv[t]->cancel_store( mvars->ctx[t] ); + mvars->state[t] = ST_CLOSED; + mvars->ret = mvars->skip = 1; + sync_chans( mvars, E_OPEN ); +} + +static void store_connected( int sts, void *aux ); static void store_listed( int sts, void *aux ); static int sync_listed_boxes( main_vars_t *mvars, box_ent_t *mbox ); static void done_sync_2_dyn( int sts, void *aux ); @@ -771,17 +794,18 @@ sync_chans( main_vars_t *mvars, int ent ) labels[M] = "M: ", labels[S] = "S: "; else labels[M] = labels[S] = ""; + for (t = 0; t < 2; t++) { + mvars->drv[t] = mvars->chan->stores[t]->driver; + mvars->ctx[t] = mvars->drv[t]->alloc_store( mvars->chan->stores[t], labels[t] ); + set_bad_callback( mvars->ctx[t], store_bad, AUX ); + } for (t = 0; ; t++) { info( "Opening %s store %s...\n", str_ms[t], mvars->chan->stores[t]->name ); - mvars->drv[t] = mvars->chan->stores[t]->driver; - mvars->drv[t]->open_store( mvars->chan->stores[t], labels[t], store_opened, AUX ); - if (t) + mvars->drv[t]->connect_store( mvars->ctx[t], store_connected, AUX ); + if (t || mvars->skip) break; - if (mvars->skip) { - mvars->state[1] = ST_CLOSED; - break; - } } + mvars->cben = 1; opened: if (mvars->skip) @@ -856,13 +880,19 @@ sync_chans( main_vars_t *mvars, int ent ) } next: + mvars->cben = 0; for (t = 0; t < 2; t++) - if (mvars->state[t] == ST_OPEN) { - mvars->drv[t]->disown_store( mvars->ctx[t] ); + if (mvars->state[t] == ST_FRESH) { + /* An unconnected store may be only cancelled. */ mvars->state[t] = ST_CLOSED; + mvars->drv[t]->cancel_store( mvars->ctx[t] ); + } else if (mvars->state[t] == ST_CONNECTED || mvars->state[t] == ST_OPEN) { + mvars->state[t] = ST_CANCELING; + mvars->drv[t]->cancel_cmds( mvars->ctx[t], cancel_prep_done, AUX ); } + mvars->cben = 1; if (mvars->state[M] != ST_CLOSED || mvars->state[S] != ST_CLOSED) { - mvars->skip = mvars->cben = 1; + mvars->skip = 1; return; } if (mvars->chanptr->boxlist == 2) { @@ -885,72 +915,64 @@ sync_chans( main_vars_t *mvars, int ent ) } static void -store_bad( void *aux ) -{ - MVARS(aux) - - mvars->drv[t]->cancel_store( mvars->ctx[t] ); - mvars->ret = mvars->skip = 1; - mvars->state[t] = ST_CLOSED; - sync_chans( mvars, E_OPEN ); -} - -static void -store_opened( store_t *ctx, void *aux ) +store_connected( int sts, void *aux ) { MVARS(aux) string_list_t *cpat; int cflags; - if (!ctx) { - mvars->ret = mvars->skip = 1; - mvars->state[t] = ST_CLOSED; - sync_chans( mvars, E_OPEN ); + switch (sts) { + case DRV_CANCELED: return; - } - mvars->ctx[t] = ctx; - if (!mvars->skip && !mvars->chanptr->boxlist && mvars->chan->patterns && !ctx->listed) { - for (cflags = 0, cpat = mvars->chan->patterns; cpat; cpat = cpat->next) { - const char *pat = cpat->string; - if (*pat != '!') { - char buf[8]; - int bufl = snprintf( buf, sizeof(buf), "%s%s", nz( mvars->chan->boxes[t], "" ), pat ); - int flags = 0; - /* Partial matches like "INB*" or even "*" are not considered, - * except implicity when the INBOX lives under Path. */ - if (starts_with( buf, bufl, "INBOX", 5 )) { - char c = buf[5]; - if (!c) { - /* User really wants the INBOX. */ - flags |= LIST_INBOX; - } else if (c == '/') { - /* Flattened sub-folders of INBOX actually end up in Path. */ - if (ctx->conf->flat_delim) + case DRV_OK: + if (!mvars->skip && !mvars->chanptr->boxlist && mvars->chan->patterns && !mvars->ctx[t]->listed) { + for (cflags = 0, cpat = mvars->chan->patterns; cpat; cpat = cpat->next) { + const char *pat = cpat->string; + if (*pat != '!') { + char buf[8]; + int bufl = snprintf( buf, sizeof(buf), "%s%s", nz( mvars->chan->boxes[t], "" ), pat ); + int flags = 0; + /* Partial matches like "INB*" or even "*" are not considered, + * except implicity when the INBOX lives under Path. */ + if (starts_with( buf, bufl, "INBOX", 5 )) { + char c = buf[5]; + if (!c) { + /* User really wants the INBOX. */ + flags |= LIST_INBOX; + } else if (c == '/') { + /* Flattened sub-folders of INBOX actually end up in Path. */ + if (mvars->ctx[t]->conf->flat_delim) + flags |= LIST_PATH; + else + flags |= LIST_INBOX; + } else { + /* User may not want the INBOX after all ... */ flags |= LIST_PATH; - else - flags |= LIST_INBOX; + /* ... but maybe he does. + * The flattened sub-folder case is implicitly covered by the previous line. */ + if (c == '*' || c == '%') + flags |= LIST_INBOX; + } } else { - /* User may not want the INBOX after all ... */ flags |= LIST_PATH; - /* ... but maybe he does. - * The flattened sub-folder case is implicitly covered by the previous line. */ - if (c == '*' || c == '%') - flags |= LIST_INBOX; } - } else { - flags |= LIST_PATH; + debug( "pattern '%s' (effective '%s'): %sPath, %sINBOX\n", + pat, buf, (flags & LIST_PATH) ? "" : "no ", (flags & LIST_INBOX) ? "" : "no "); + cflags |= flags; } - debug( "pattern '%s' (effective '%s'): %sPath, %sINBOX\n", - pat, buf, (flags & LIST_PATH) ? "" : "no ", (flags & LIST_INBOX) ? "" : "no "); - cflags |= flags; } + mvars->state[t] = ST_CONNECTED; + mvars->drv[t]->list_store( mvars->ctx[t], cflags, store_listed, AUX ); + return; } - set_bad_callback( ctx, store_bad, AUX ); - mvars->drv[t]->list_store( ctx, cflags, store_listed, AUX ); - } else { mvars->state[t] = ST_OPEN; - sync_chans( mvars, E_OPEN ); + break; + default: + mvars->ret = mvars->skip = 1; + mvars->state[t] = ST_OPEN; + break; } + sync_chans( mvars, E_OPEN ); } static void diff --git a/src/socket.c b/src/socket.c index 49586ac..27bc8cd 100644 --- a/src/socket.c +++ b/src/socket.c @@ -516,6 +516,7 @@ socket_connected( conn_t *conn ) { #ifdef HAVE_IPV6 freeaddrinfo( conn->addrs ); + conn->addrs = 0; #endif conf_notifier( &conn->notify, 0, POLLIN ); socket_expect_read( conn, 0 ); @@ -528,6 +529,7 @@ socket_connect_bail( conn_t *conn ) { #ifdef HAVE_IPV6 freeaddrinfo( conn->addrs ); + conn->addrs = 0; #endif free( conn->name ); conn->name = 0; @@ -543,6 +545,12 @@ socket_close( conn_t *sock ) socket_close_internal( sock ); free( sock->name ); sock->name = 0; +#ifdef HAVE_IPV6 + if (sock->addrs) { + freeaddrinfo( sock->addrs ); + sock->addrs = 0; + } +#endif #ifdef HAVE_LIBSSL if (sock->ssl) { SSL_free( sock->ssl );