net_imap: Explicitly stop idling before remote MOVE/COPY operations.

In some circumstances, attempts to move/copy messages between
different IMAP servers will fail, with the response to the
issued MOVE/COPY command being the end of IDLE confirmation,
suggesting that we attempted to run a command without first
terminating an in-progress IDLE. Now, if the target mailbox's
client is still idling when processing the relevant command,
explicitly stop idling first.

Related enhancements:

* net_imap: Reduce Yandex IDLE time from 2 minutes to around 1 minute
  to more reliably prevent connections from being prematurely
  disconnected.
* net_imap: Store and display age of proxied IMAP clients.
* net_ssh: Add missing break statement, to end session if SSH_ERROR
  is returned during shutdown.
* io_tls: Add universal debug messages when TLS session list is rebuilt.
* socket.c: Add comment clarifying when -b flag may be required.

LBBS-76 #close

bbs/socket.c

@@ -245,6 +245,12 @@ static int __bbs_make_ip_socket(int *sock, int port, int type, const char *ip, c
 			break;
 		}
 
+		/* The reason we pass option_rebind again instead of just 1 to always force rebind
+		 * at this point is if we don't request reuse initially, it doesn't work if we
+		 * ask for it now. And, per the comment above, it's not always ideal to
+		 * ask for it initially either.
+		 * Thus the only elegant way to ensure we always rebind successfully is if the
+		 * BBS is started with the -b option. */
 		res = __bbs_socket_bind(sock, option_rebind, type, port, ip, interface, file, line, func);
 		if (!option_rebind) {
 			/* If we don't require reuse, try once and then give up */

io/io_tls.c

@@ -351,6 +351,7 @@ static void *ssl_io_thread(void *unused)
 				bbs_debug(4, "SSL I/O thread has been instructed to exit\n");
 				break; /* We're shutting down. */
 			}
+			bbs_debug(8, "Rebuilding TLS session list\n");
 			needcreate = 0;
 			free_if(pfds);
 			free_if(ssl_list);
@@ -412,6 +413,8 @@ static void *ssl_io_thread(void *unused)
 					snprintf(tmpbuf, sizeof(tmpbuf), ", %d dead", numdead);
 				}
 				bbs_debug(7, "SSL I/O thread now polling %d -> %d fd%s (%d connection%s%s)\n", oldnumfds, numfds, ESS(numfds), numssl, ESS(numssl), tmpbuf);
+			} else {
+				bbs_debug(7, "SSL I/O thread still polling %d fd%s\n", numfds, ESS(numfds));
 			}
 			prevfds = numfds;
 		}

nets/net_imap.c

@@ -270,15 +270,17 @@ static int cli_imap_clients(struct bbs_cli_args *a)
 	struct imap_session *imap;
 	time_t now = time(NULL);
 
-	bbs_dprintf(a->fdout, "%4s %-25s %6s %4s %9s\n", "Node", "Name", "Active", "Dead", "Idle");
+	bbs_dprintf(a->fdout, "%4s %-25s %6s %4s %9s %s\n", "Node", "Name", "Active", "Dead", "Idle", "Age");
 	RWLIST_RDLOCK(&sessions);
 	RWLIST_TRAVERSE(&sessions, imap, entry) {
 		struct imap_client *client;
 		RWLIST_RDLOCK(&imap->clients);
 		RWLIST_TRAVERSE(&imap->clients, client, entry) {
+			char elapsed[24];
 			time_t idle_elapsed = now - client->idlestarted;
-			bbs_dprintf(a->fdout, "%4u %-25s %6s %4s %4lu/%4d\n",
-				imap->node->id, client->name, BBS_YN(client->active), BBS_YN(client->dead), client->idling ? idle_elapsed : 0, client->maxidlesec);
+			print_time_elapsed(client->created, now, elapsed, sizeof(elapsed));
+			bbs_dprintf(a->fdout, "%4u %-25s %6s %4s %4lu/%4d %s\n",
+				imap->node->id, client->name, BBS_YN(client->active), BBS_YN(client->dead), client->idling ? idle_elapsed : 0, client->maxidlesec, elapsed);
 		}
 		RWLIST_UNLOCK(&imap->clients);
 	}
@@ -2785,7 +2787,12 @@ static int handle_remote_move(struct imap_session *imap, char *dest, const char
 	/* No setup needed for source, since that mailbox is already selected.
 	 * No setup needed for dest, either, since the APPEND mailbox doesn't need to be selected. */
 
-	if (!destclient) {
+	if (destclient) {
+		/* If we're idling on this client currently, stop */
+		if (destclient->idling) {
+			imap_client_idle_stop(destclient);
+		}
+	} else {
 		/* Ensure we have sufficient ACLs for the destination folder */
 		IMAP_REQUIRE_ACL(traversalstack.acl, IMAP_ACL_INSERT);
 	}

nets/net_imap/imap.h

@@ -29,6 +29,7 @@ struct imap_client {
 	int virtcapabilities;		/* Capabilities of remote IMAP server */
 	char virtdelimiter;			/* Hierarchy delimiter used by remote server */
 	char *virtlist;				/* Result of LIST-STATUS command on remote IMAP server */
+	time_t created;				/* Time client was created */
 	time_t virtlisttime;		/* Time that LIST-STATUS command was run */
 	time_t lastactive;			/* Last active time */
 	char *bgmailbox;			/* Mail for background IDLE */

nets/net_imap/imap_client.c

@@ -394,6 +394,7 @@ static struct imap_client *client_new(const char *name)
 	}
 
 	strcpy(client->data, name); /* Safe */
+	client->created = time(NULL);
 	client->virtprefix = client->name = client->data;
 	client->virtprefixlen = len;
 	client->client.fd = -1;
@@ -548,7 +549,10 @@ int __imap_client_send_wait_response(struct imap_client *client, int fd, int ms,
 	UNUSED(lineno);
 #endif
 
-	if (client->idling) {
+	/* Only include this check if it's not the active client,
+	 * since a lot of pass through command code uses imap_client_send,
+	 * in which case this would be a false positive otherwise. */
+	if (client != client->imap->client && client->idling) {
 		bbs_warning("Client is currently idling while attempting to write '%s%s'", tagbuf, buf);
 		bbs_soft_assert(0); /* Could stop idle now if this were to happen, but that would just mask a bug */
 	}
@@ -803,7 +807,12 @@ struct imap_client *__imap_client_get_by_url(struct imap_session *imap, const ch
 	 * so it's good to keep alive if possible...
 	 */
 	if (!strcmp(url.host, "imap.yandex.com")) {
-		client->maxidlesec = 120; /* 2 minutes */
+		/* Even 2 minutes doesn't seem to suffice, 1 minute seems to work a lot better.
+		 * Even as short as 75 seconds, clients start dropping like flies pretty quickly.
+		 * At 60 seconds, it's much more consistent. Why Yandex wants increased network
+		 * traffic for no reason at all, I don't know... but we have to do what we have to do
+		 * to keep these connections alive... */
+		client->maxidlesec = 65; /* ~1 minute */
 	} else {
 		client->maxidlesec = 1800; /* 30 minutes */
 	}

nets/net_ssh.c

@@ -917,6 +917,7 @@ static void handle_session(ssh_event event, ssh_session session)
 			if (res == SSH_ERROR) {
 				int code = ssh_channel_get_exit_status(sdata.channel);
 				bbs_error("SSH session ended uncleanly with code %d\n", code);
+				break;
 			} else if (res == SSH_EOF) {
 				bbs_debug(3, "Received EOF\n");
 				break;