lib-http: client: Reworked connection close handling.

Now, the peer is immediately notified of the lost connection.
Before, this step was only taken when the connection was fully dereferenced.
To prevent recursive notifications between peer and connection, handling the loss of a connection is deferred to the request handler.
When a peer is freed, any associated lingering connections have conn->peer set to NULL.

src/lib-http/http-client-connection.c

@@ -46,8 +46,6 @@ http_client_connection_debug(struct http_client_connection *conn,
 
 static void http_client_connection_ready(struct http_client_connection *conn);
 static void http_client_connection_input(struct connection *_conn);
-static void
-http_client_connection_disconnect(struct http_client_connection *conn);
 
 unsigned int
 http_client_connection_count_pending(struct http_client_connection *conn)
@@ -561,8 +559,10 @@ static void http_client_payload_destroyed(struct http_client_request *req)
 	   somewhere from the API user's code, which we can't really know what
 	   state it is in). this call also triggers sending a new request if
 	   necessary. */
-	conn->to_input =
-		timeout_add_short(0, http_client_payload_destroyed_timeout, conn);
+	if (!conn->disconnected) {
+		conn->to_input = timeout_add_short
+			(0, http_client_payload_destroyed_timeout, conn);
+	}
 
 	/* room for new requests */
 	if (http_client_connection_is_ready(conn))
@@ -609,7 +609,8 @@ http_client_connection_return_response(
 	http_client_connection_ref(conn);
 	retrying = !http_client_request_callback(req, response);
 	tmp_conn = conn;
-	if (!http_client_connection_unref(&tmp_conn)) {
+	if (!http_client_connection_unref(&tmp_conn) ||
+		conn->disconnected) {
 		/* the callback managed to get this connection destroyed */
 		if (!retrying)
 			http_client_request_finish(req);
@@ -1417,6 +1418,16 @@ void http_client_connection_ref(struct http_client_connection *conn)
 static void
 http_client_connection_disconnect(struct http_client_connection *conn)
 {
+	struct http_client_peer *peer = conn->peer;
+	ARRAY_TYPE(http_client_connection) *conn_arr;
+	struct http_client_connection *const *conn_idx;
+
+	if (conn->disconnected)
+		return;
+	conn->disconnected = TRUE;
+
+	http_client_connection_debug(conn, "Connection disconnect");
+
 	conn->closing = TRUE;
 	conn->connected = FALSE;
 
@@ -1450,14 +1461,23 @@ http_client_connection_disconnect(struct http_client_connection *conn)
 		timeout_remove(&conn->to_idle);
 	if (conn->to_response != NULL)
 		timeout_remove(&conn->to_response);
+
+	/* remove this connection from the list */
+	conn_arr = &peer->conns;
+	array_foreach(conn_arr, conn_idx) {
+		if (*conn_idx == conn) {
+			array_delete(conn_arr, array_foreach_idx(conn_arr, conn_idx), 1);
+			break;
+		}
+	}
+
+	if (conn->connect_succeeded)
+		http_client_peer_connection_lost(peer);
 }
 
 bool http_client_connection_unref(struct http_client_connection **_conn)
 {
 	struct http_client_connection *conn = *_conn;
-	struct http_client_connection *const *conn_idx;
-	ARRAY_TYPE(http_client_connection) *conn_arr;
-	struct http_client_peer *peer = conn->peer;
 
 	i_assert(conn->refcount > 0);
 
@@ -1470,6 +1490,13 @@ bool http_client_connection_unref(struct http_client_connection **_conn)
 
 	http_client_connection_disconnect(conn);
 
+	i_assert(conn->io_req_payload == NULL);
+	i_assert(conn->to_requests == NULL);
+	i_assert(conn->to_connect == NULL);
+	i_assert(conn->to_input == NULL);
+	i_assert(conn->to_idle == NULL);
+	i_assert(conn->to_response == NULL);
+
 	if (array_is_created(&conn->request_wait_list))
 		array_free(&conn->request_wait_list);
 
@@ -1478,17 +1505,7 @@ bool http_client_connection_unref(struct http_client_connection **_conn)
 	if (conn->connect_initialized)
 		connection_deinit(&conn->conn);
 
-	/* remove this connection from the list */
-	conn_arr = &conn->peer->conns;
-	array_foreach(conn_arr, conn_idx) {
-		if (*conn_idx == conn) {
-			array_delete(conn_arr, array_foreach_idx(conn_arr, conn_idx), 1);
-			break;
-		}
-	}
-
-	if (conn->connect_succeeded)
-		http_client_peer_connection_lost(peer);
+	i_free(conn->label);
 	i_free(conn);
 	return FALSE;
 }
@@ -1504,6 +1521,17 @@ void http_client_connection_close(struct http_client_connection **_conn)
 	http_client_connection_unref(_conn);
 }
 
+void http_client_connection_peer_closed(struct http_client_connection **_conn)
+{
+	struct http_client_connection *conn = *_conn;
+
+	http_client_connection_debug(conn, "Peer closed");
+	http_client_connection_disconnect(conn);
+
+	if (http_client_connection_unref(_conn))
+		conn->peer = NULL;
+}
+
 void http_client_connection_switch_ioloop(struct http_client_connection *conn)
 {
 	if (conn->io_req_payload != NULL)

src/lib-http/http-client-peer.c

@@ -551,7 +551,7 @@ http_client_peer_disconnect(struct http_client_peer *peer)
 	t_array_init(&conns, array_count(&peer->conns));
 	array_copy(&conns.arr, 0, &peer->conns.arr, 0, array_count(&peer->conns));
 	array_foreach_modifiable(&conns, conn) {
-		http_client_connection_unref(conn);
+		http_client_connection_peer_closed(conn);
 	}
 	i_assert(array_count(&peer->conns) == 0);
 
@@ -764,15 +764,8 @@ void http_client_peer_connection_lost(struct http_client_peer *peer)
 		return;
 	}
 
-	/* check if peer is still relevant */
-	if (array_count(&peer->conns) == 0 &&
-		http_client_peer_requests_pending(peer, &num_urgent) == 0) {
-		http_client_peer_close(&peer);
-		return;
-	}
-
 	/* if there are pending requests for this peer, create a new connection
-	   for them. */
+	   for them. if not, this peer will wind itself down. */
 	http_client_peer_trigger_request_handler(peer);
 }
 

src/lib-http/http-client-private.h

@@ -171,6 +171,7 @@ struct http_client_connection {
 	unsigned int connect_initialized:1; /* connection was initialized */
 	unsigned int connect_succeeded:1;
 	unsigned int closing:1;
+	unsigned int disconnected:1;
 	unsigned int close_indicated:1;
 	unsigned int output_locked:1;       /* output is locked; no pipelining */
 	unsigned int output_broken:1;       /* output is broken; no more requests */
@@ -394,6 +395,9 @@ void http_client_connection_ref(struct http_client_connection *conn);
 /* Returns FALSE if unrefing destroyed the connection entirely */
 bool http_client_connection_unref(struct http_client_connection **_conn);
 void http_client_connection_close(struct http_client_connection **_conn);
+
+void http_client_connection_peer_closed(struct http_client_connection **_conn);
+
 int http_client_connection_output(struct http_client_connection *conn);
 void http_client_connection_start_request_timeout(
 	struct http_client_connection *conn);