add socket timeout handling

NEWS

@@ -1,3 +1,7 @@
+[1.3.0]
+
+Network timeout handling has been added.
+
 [1.2.0]
 
 The 'isync' compatibility wrapper is now deprecated.

TODO

@@ -8,8 +8,7 @@ won't cause the same error message for every attached store.
 
 make SSL (connect) timeouts produce a bit more than "Unidentified socket error".
 
-network timeout handling in general would be a good idea.
-lock timeout handling, too.
+uidvalidity lock timeout handling would be a good idea.
 
 add message expiration based on arrival date (message date would be too
 unreliable). MaxAge; probably mutually exclusive to MaxMessages.

src/drv_imap.c

@@ -319,6 +319,7 @@ send_imap_cmd( imap_store_t *ctx, struct imap_cmd *cmd )
 	*ctx->in_progress_append = cmd;
 	ctx->in_progress_append = &cmd->next;
 	ctx->num_in_progress++;
+	socket_expect_read( &ctx->conn, 1 );
 	return 0;
 
   bail:
@@ -371,6 +372,7 @@ cancel_submitted_imap_cmds( imap_store_t *ctx )
 {
 	struct imap_cmd *cmd;
 
+	socket_expect_read( &ctx->conn, 0 );
 	while ((cmd = ctx->in_progress)) {
 		ctx->in_progress = cmd->next;
 		/* don't update num_in_progress and in_progress_append - store is dead */
@@ -1316,6 +1318,7 @@ imap_socket_read( void *aux )
 			error( "IMAP error: unexpected reply: %s %s\n", arg, cmd ? cmd : "" );
 			break; /* this may mean anything, so prefer not to spam the log */
 		} else if (*arg == '+') {
+			socket_expect_read( &ctx->conn, 0 );
 			/* There can be any number of commands in flight, but only the last
 			 * one can require a continuation, as it enforces a round-trip. */
 			cmdp = (struct imap_cmd *)((char *)ctx->in_progress_append -
@@ -1340,6 +1343,7 @@ imap_socket_read( void *aux )
 				error( "IMAP error: unexpected command continuation request\n" );
 				break;
 			}
+			socket_expect_read( &ctx->conn, 1 );
 		} else {
 			tag = atoi( arg );
 			for (pcmdp = &ctx->in_progress; (cmdp = *pcmdp); pcmdp = &cmdp->next)
@@ -1350,7 +1354,8 @@ imap_socket_read( void *aux )
 		  gottag:
 			if (!(*pcmdp = cmdp->next))
 				ctx->in_progress_append = pcmdp;
-			ctx->num_in_progress--;
+			if (!--ctx->num_in_progress)
+				socket_expect_read( &ctx->conn, 0 );
 			arg = next_arg( &cmd );
 			if (!arg) {
 				error( "IMAP error: malformed tagged response\n" );
@@ -1614,6 +1619,8 @@ imap_open_store_connected( int ok, void *aux )
 	else if (srvc->ssl_type == SSL_IMAPS)
 		socket_start_tls( &ctx->conn, imap_open_store_tlsstarted1 );
 #endif
+	else
+		socket_expect_read( &ctx->conn, 1 );
 }
 
 #ifdef HAVE_LIBSSL
@@ -1624,12 +1631,15 @@ imap_open_store_tlsstarted1( int ok, void *aux )
 
 	if (!ok)
 		imap_open_store_ssl_bail( ctx );
+	else
+		socket_expect_read( &ctx->conn, 1 );
 }
 #endif
 
 static void
 imap_open_store_greeted( imap_store_t *ctx )
 {
+	socket_expect_read( &ctx->conn, 0 );
 	if (!ctx->caps)
 		imap_exec( ctx, 0, imap_open_store_p2, "CAPABILITY" );
 	else
@@ -2694,6 +2704,7 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 	} else
 		return 0;
 
+	server->sconf.timeout = 20;
 #ifdef HAVE_LIBSSL
 	server->ssl_type = -1;
 	server->sconf.ssl_versions = -1;
@@ -2729,6 +2740,8 @@ imap_parse_store( conffile_t *cfg, store_conf_t **storep )
 			server->pass_cmd = nfstrdup( cfg->val );
 		else if (!strcasecmp( "Port", cfg->cmd ))
 			server->sconf.port = parse_int( cfg );
+		else if (!strcasecmp( "Timeout", cfg->cmd ))
+			server->sconf.timeout = parse_int( cfg );
 		else if (!strcasecmp( "PipelineDepth", cfg->cmd )) {
 			if ((server->max_in_progress = parse_int( cfg )) < 1) {
 				error( "%s:%d: PipelineDepth must be at least 1\n", cfg->file, cfg->line );

src/mbsync.1

@@ -277,6 +277,12 @@ Specify the TCP port number of the IMAP server.  (Default: 143 for IMAP,
 If \fBTunnel\fR is used, this setting is ignored.
 ..
 .TP
+\fBTimeout\fR \fItimeout\fR
+Specify the connect and data timeout for the IMAP server in seconds.
+Zero means unlimited.
+(Default: \fI20\fR)
+..
+.TP
 \fBUser\fR \fIusername\fR
 Specify the login name on the IMAP server.
 ..

src/socket.c

@@ -260,6 +260,7 @@ socket_start_tls( conn_t *conn, void (*cb)( int ok, void *aux ) )
 	conn->ssl = SSL_new( ((server_conf_t *)conn->conf)->SSLContext );
 	SSL_set_fd( conn->ssl, conn->fd );
 	SSL_set_mode( conn->ssl, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER );
+	socket_expect_read( conn, 1 );
 	conn->state = SCK_STARTTLS;
 	start_tls_p2( conn );
 }
@@ -279,6 +280,7 @@ start_tls_p2( conn_t *conn )
 
 static void start_tls_p3( conn_t *conn, int ok )
 {
+	socket_expect_read( conn, 0 );
 	conn->state = SCK_READY;
 	conn->callbacks.starttls( ok, conn->callback_aux );
 }
@@ -324,6 +326,7 @@ socket_start_deflate( conn_t *conn )
 
 static void socket_fd_cb( int, void * );
 static void socket_fake_cb( void * );
+static void socket_timeout_cb( void * );
 
 static void socket_connect_one( conn_t * );
 static void socket_connect_failed( conn_t * );
@@ -337,6 +340,7 @@ socket_open_internal( conn_t *sock, int fd )
 	fcntl( fd, F_SETFL, O_NONBLOCK );
 	init_notifier( &sock->notify, fd, socket_fd_cb, sock );
 	init_wakeup( &sock->fd_fake, socket_fake_cb, sock );
+	init_wakeup( &sock->fd_timeout, socket_timeout_cb, sock );
 }
 
 static void
@@ -344,6 +348,7 @@ socket_close_internal( conn_t *sock )
 {
 	wipe_notifier( &sock->notify );
 	wipe_wakeup( &sock->fd_fake );
+	wipe_wakeup( &sock->fd_timeout );
 	close( sock->fd );
 	sock->fd = -1;
 }
@@ -482,6 +487,7 @@ socket_connect_one( conn_t *sock )
 			return;
 		}
 		conf_notifier( &sock->notify, 0, POLLOUT );
+		socket_expect_read( sock, 1 );
 		sock->state = SCK_CONNECTING;
 		info( "\v\n" );
 		return;
@@ -512,6 +518,7 @@ socket_connected( conn_t *conn )
 	freeaddrinfo( conn->addrs );
 #endif
 	conf_notifier( &conn->notify, 0, POLLIN );
+	socket_expect_read( conn, 0 );
 	conn->state = SCK_READY;
 	conn->callbacks.connect( 1, conn->callback_aux );
 }
@@ -579,6 +586,8 @@ do_read( conn_t *sock, char *buf, int len )
 	int n;
 
 	assert( sock->fd >= 0 );
+	if (pending_wakeup( &sock->fd_timeout ))
+		conf_wakeup( &sock->fd_timeout, sock->conf->timeout );
 #ifdef HAVE_LIBSSL
 	if (sock->ssl) {
 		if ((n = ssl_return( "read from", sock, SSL_read( sock->ssl, buf, len ) )) <= 0)
@@ -662,6 +671,13 @@ socket_fill( conn_t *sock )
 	}
 }
 
+void
+socket_expect_read( conn_t *conn, int expect )
+{
+	if (conn->conf->timeout > 0 && expect != pending_wakeup( &conn->fd_timeout ))
+		conf_wakeup( &conn->fd_timeout, expect ? conn->conf->timeout : -1 );
+}
+
 int
 socket_read( conn_t *conn, char *buf, int len )
 {
@@ -970,6 +986,20 @@ socket_fake_cb( void *aux )
 		do_queued_write( conn );
 }
 
+static void
+socket_timeout_cb( void *aux )
+{
+	conn_t *conn = (conn_t *)aux;
+
+	if (conn->state == SCK_CONNECTING) {
+		errno = ETIMEDOUT;
+		socket_connect_failed( conn );
+	} else {
+		error( "Socket error on %s: timeout.\n", conn->name );
+		socket_fail( conn );
+	}
+}
+
 #ifdef HAVE_LIBZ
 static void
 z_fake_cb( void *aux )

src/socket.h

@@ -47,6 +47,7 @@ typedef struct server_conf {
 	char *tunnel;
 	char *host;
 	int port;
+	int timeout;
 #ifdef HAVE_LIBSSL
 	char *cert_file;
 	char system_certs;
@@ -96,6 +97,7 @@ typedef struct {
 
 	notifier_t notify;
 	wakeup_t fd_fake;
+	wakeup_t fd_timeout;
 
 	/* writing */
 	buff_chunk_t *append_buf; /* accumulating buffer */
@@ -137,6 +139,7 @@ void socket_connect( conn_t *conn, void (*cb)( int ok, void *aux ) );
 void socket_start_tls(conn_t *conn, void (*cb)( int ok, void *aux ) );
 void socket_start_deflate( conn_t *conn );
 void socket_close( conn_t *sock );
+void socket_expect_read( conn_t *sock, int expect );
 int socket_read( conn_t *sock, char *buf, int len ); /* never waits */
 char *socket_read_line( conn_t *sock ); /* don't free return value; never waits */
 typedef enum { KeepOwn = 0, GiveOwn } ownership_t;