imap: When hibernating, wait for old imap process to cleanup before c…

…reating new ones.

This fixes stats errors:

stats: Error: FIFO input error: CONNECT: Duplicate session ID Y7Q6E4U7xO1/AAAB for user testuser service imap
stats: Warning: Couldn't find session ID: Y7Q6E4U7xO1/AAAB

src/imap-hibernate/imap-hibernate-client.c

@@ -16,6 +16,7 @@ struct imap_hibernate_client {
 	struct connection conn;
 	struct imap_client *imap_client;
 	bool imap_client_created;
+	bool finished;
 	bool debug;
 };
 
@@ -34,6 +35,8 @@ static void imap_hibernate_client_destroy(struct connection *conn)
 
 	if (!client->imap_client_created)
 		master_service_client_connection_destroyed(master_service);
+	else if (client->finished)
+		imap_client_create_finish(client->imap_client);
 	connection_deinit(conn);
 	i_free(conn);
 }
@@ -177,6 +180,10 @@ imap_hibernate_client_input_line(struct connection *conn, const char *line)
 		conn->version_received = TRUE;
 		return 1;
 	}
+	if (client->finished) {
+		i_error("Received unexpected line: %s", line);
+		return -1;
+	}
 
 	if (client->imap_client == NULL) {
 		char *const *args;
@@ -219,15 +226,14 @@ imap_hibernate_client_input_line(struct connection *conn, const char *line)
 	} else if (ret == 0) {
 		/* still need to read another fd */
 		i_stream_unix_set_read_fd(conn->input);
-		o_stream_send_str(conn->output, "+\n");
-		return 1;
 	} else {
-		/* finished - always disconnect the hibernate client
-		   afterwards */
-		o_stream_send_str(conn->output, "+\n");
-		imap_client_create_finish(client->imap_client);
-		return -1;
+		/* finished - wait for disconnection from imap before finishing.
+		   this way the old imap process will have time to destroy
+		   itself before we have a chance to create another one. */
+		client->finished = TRUE;
 	}
+	o_stream_send_str(conn->output, "+\n");
+	return 1;
 }
 
 void imap_hibernate_client_create(int fd, bool debug)

src/imap/imap-client-hibernate.c

@@ -138,7 +138,7 @@ static int imap_hibernate_process_read(int fd, const char *path)
 
 static int
 imap_hibernate_process_send(struct client *client,
-			    const buffer_t *state, int fd_notify)
+			    const buffer_t *state, int fd_notify, int *fd_r)
 {
 	string_t *cmd = t_str_new(512);
 	const char *path;
@@ -147,6 +147,8 @@ imap_hibernate_process_send(struct client *client,
 
 	i_assert(state->used > 0);
 
+	*fd_r = -1;
+
 	path = t_strconcat(client->user->set->base_dir,
 			   "/"IMAP_HIBERNATE_SOCKET_NAME, NULL);
 	fd = net_connect_unix_with_retries(path, 1000);
@@ -169,16 +171,20 @@ imap_hibernate_process_send(struct client *client,
 			ret = imap_hibernate_process_read(fd, path);
 	}
 	alarm(0);
-	net_disconnect(fd);
-	return ret < 0 ? -1 : 0;
+	if (ret < 0) {
+		net_disconnect(fd);
+		return -1;
+	}
+	*fd_r = fd;
+	return 0;
 }
 
 bool imap_client_hibernate(struct client **_client)
 {
 	struct client *client = *_client;
 	buffer_t *state;
 	const char *error;
-	int ret, fd_notify = -1;
+	int ret, fd_notify = -1, fd_hibernate = -1;
 
 	if (client->fd_in != client->fd_out) {
 		/* we won't try to hibernate stdio clients */
@@ -215,7 +221,7 @@ bool imap_client_hibernate(struct client **_client)
 		}
 	}
 	if (ret > 0) {
-		if (imap_hibernate_process_send(client, state, fd_notify) < 0)
+		if (imap_hibernate_process_send(client, state, fd_notify, &fd_hibernate) < 0)
 			ret = -1;
 	}
 	if (fd_notify != -1)
@@ -227,6 +233,12 @@ bool imap_client_hibernate(struct client **_client)
 		client_destroy(client, NULL);
 		*_client = NULL;
 	}
+	/* notify imap-hibernate that we're done by closing the connection.
+	   do this only after client is destroyed. this way imap-hibernate
+	   won't try to launch another imap process too early and cause
+	   problems (like sending duplicate session ID to stats process) */
+	if (fd_hibernate != -1)
+		net_disconnect(fd_hibernate);
 	buffer_free(&state);
 	return ret > 0;
 }