dovecot-2.2: lib-http: Fixed hangs with urgent requests.

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-2.2/rev/d3e3edf8f7eb
changeset: 16005:d3e3edf8f7eb
user:      Timo Sirainen <tss at iki.fi>
date:      Sun Mar 10 17:19:14 2013 +0200
description:
lib-http: Fixed hangs with urgent requests.

diffstat:

 src/lib-http/http-client-connection.c |  24 ++++++++++++++++--------
 src/lib-http/http-client-peer.c       |  26 +++++++++++++++-----------
 2 files changed, 31 insertions(+), 19 deletions(-)

diffs (137 lines):

diff -r 5e63f628e3ca -r d3e3edf8f7eb src/lib-http/http-client-connection.c
--- a/src/lib-http/http-client-connection.c	Sun Mar 10 17:17:47 2013 +0200
+++ b/src/lib-http/http-client-connection.c	Sun Mar 10 17:19:14 2013 +0200
@@ -61,9 +61,13 @@
 
 bool http_client_connection_is_ready(struct http_client_connection *conn)
 {
+	unsigned int pending_count = array_count(&conn->request_wait_list);
+
+	if (conn->pending_request != NULL)
+		pending_count++;
 	return (conn->connected && !conn->output_locked &&
-		!conn->close_indicated &&	array_count(&conn->request_wait_list) <
-			conn->client->set.max_pipelined_requests);
+		!conn->close_indicated &&
+		pending_count < conn->client->set.max_pipelined_requests);
 }
 
 bool http_client_connection_is_idle(struct http_client_connection *conn)
@@ -224,6 +228,7 @@
 {
 	struct http_client_request *req = NULL;
 	const char *error;
+	bool have_pending_requests;
 
 	if (!http_client_connection_is_ready(conn)) {
 		http_client_connection_debug(conn, "Not ready for next request");
@@ -231,8 +236,9 @@
 	}
 
 	/* claim request, but no urgent request can be second in line */
-	req = http_client_peer_claim_request(conn->peer,
-		array_count(&conn->request_wait_list) > 0); 
+	have_pending_requests = array_count(&conn->request_wait_list) > 0 ||
+		conn->pending_request != NULL;
+	req = http_client_peer_claim_request(conn->peer, have_pending_requests);
 	if (req == NULL) {
 		http_client_connection_check_idle(conn);
 		return FALSE;	
@@ -324,10 +330,12 @@
 	http_client_connection_input(&conn->conn);
 }
 
-static void http_client_payload_destroyed(struct http_client_connection *conn)
+static void http_client_payload_destroyed(struct http_client_request *req)
 {
+	struct http_client_connection *conn = req->conn;
+
+	i_assert(conn->pending_request == req);
 	i_assert(conn->incoming_payload != NULL);
-	i_assert(conn->pending_request != NULL);
 	i_assert(conn->conn.io == NULL);
 
 	http_client_connection_debug(conn, "Response payload stream destroyed");
@@ -338,7 +346,7 @@
 
 	conn->incoming_payload = NULL;
 
-	http_client_request_finish(&conn->pending_request);
+	http_client_request_finish(&req);
 	conn->pending_request = NULL;
 
 	/* input stream may have pending input. make sure input handler
@@ -368,7 +376,7 @@
 			i_stream_create_limit(response->payload, (uoff_t)-1);
 		i_stream_set_destroy_callback(response->payload,
 					      http_client_payload_destroyed,
-					      conn);
+					      req);
 		/* the callback may add its own I/O, so we need to remove
 		   our one before calling it */
 		io_remove(&conn->conn.io);
diff -r 5e63f628e3ca -r d3e3edf8f7eb src/lib-http/http-client-peer.c
--- a/src/lib-http/http-client-peer.c	Sun Mar 10 17:17:47 2013 +0200
+++ b/src/lib-http/http-client-peer.c	Sun Mar 10 17:19:14 2013 +0200
@@ -133,14 +133,15 @@
 	unsigned int connecting = 0, closing = 0, min_waiting = UINT_MAX;
 	unsigned int num_urgent, new_connections;
 
-	/* at this point we already know that a request for this peer is pending
-	 */
-	(void)http_client_peer_requests_pending(peer, &num_urgent);
+	if (http_client_peer_requests_pending(peer, &num_urgent) == 0)
+		return FALSE;
 
 	/* find the least busy connection */
 	array_foreach(&peer->conns, conn_idx) {
 		if (http_client_connection_is_ready(*conn_idx)) {
 			unsigned int waiting = array_count(&(*conn_idx)->request_wait_list);
+			if ((*conn_idx)->pending_request != NULL)
+				waiting++;
 			if (waiting < min_waiting) {
 				min_waiting = waiting;
 				conn = *conn_idx;
@@ -315,8 +316,7 @@
 
 	if (!exists)
 		array_append(&peer->hosts, &host, 1);
-	if (exists || array_count(&peer->hosts) > 1)
-		(void)http_client_peer_next_request(peer);
+	http_client_peer_handle_requests(peer);
 }
 
 struct http_client_request *
@@ -340,9 +340,14 @@
 {
 	struct http_client_host *const *host;
 
+	i_assert(array_count(&peer->conns) > 0);
+
 	http_client_peer_debug(peer, "Failed to make connection");
 
 	if (array_count(&peer->conns) == 1) {
+		/* this was the only/last connection and connecting to it
+		   failed. a second connect will probably also fail, so just
+		   abort all requests. */
 		array_foreach(&peer->hosts, host) {
 			http_client_host_connection_failure(*host, &peer->addr);
 		}
@@ -359,12 +364,11 @@
 	http_client_peer_debug(peer, "Lost a connection (%d connections left)",
 		array_count(&peer->conns));
 
-	if (array_count(&peer->conns) == 0) {
-		if (!http_client_peer_next_request(peer)) {
-			if (http_client_peer_requests_pending(peer, &num_urgent) == 0)
-				http_client_peer_free(&peer);
-		}
-	}
+	/* if there are pending requests, create a new connection for them. */
+	http_client_peer_handle_requests(peer);
+	if (array_count(&peer->conns) == 0 &&
+	    http_client_peer_requests_pending(peer, &num_urgent) == 0)
+		http_client_peer_free(&peer);
 }
 
 unsigned int http_client_peer_idle_connections(struct http_client_peer *peer)