changeset 19840:4487e5892f41

lib-http: client: Fixed crash happening when connection hits an error while handling a new request. At this point the connection dies within the peer request handler loop. Before, the dying connection could free the peer when it ended up being obsolete. But because it was still in the loop, a segfault occurred as the loop continued. Fixed by deferring the the deallocation of the peer from the connection_lost() function to the request handler loop itself if it is active.
author Stephan Bosch <stephan@rename-it.nl>
date Thu, 25 Feb 2016 22:46:11 +0100
parents ea779d90c88d
children 0101120789fb
files src/lib-http/http-client-peer.c src/lib-http/http-client-private.h
diffstat 2 files changed, 37 insertions(+), 14 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-http/http-client-peer.c	Thu Feb 25 01:18:06 2016 +0200
+++ b/src/lib-http/http-client-peer.c	Thu Feb 25 22:46:11 2016 +0100
@@ -200,15 +200,6 @@
 	i_assert(array_count(&peer->conns) == 0);
 }
 
-static void http_client_peer_check_idle(struct http_client_peer *peer)
-{
-	struct http_client_connection *const *conn_idx;
-
-	array_foreach(&peer->conns, conn_idx) {
-		http_client_connection_check_idle(*conn_idx);
-	}
-}
-
 static unsigned int
 http_client_peer_requests_pending(struct http_client_peer *peer,
 				  unsigned int *num_urgent_r)
@@ -226,6 +217,24 @@
 	return num_requests;
 }
 
+static void http_client_peer_check_idle(struct http_client_peer *peer)
+{
+	struct http_client_connection *const *conn_idx;
+	unsigned int num_urgent = 0;
+
+	if (array_count(&peer->conns) == 0 &&
+		http_client_peer_requests_pending(peer, &num_urgent) == 0) {
+		/* no connections or pending requests; die immediately */
+		http_client_peer_free(&peer);
+		return;
+	}
+
+	/* check all connections for idle status */
+	array_foreach(&peer->conns, conn_idx) {
+		http_client_connection_check_idle(*conn_idx);
+	}
+}
+
 static void
 http_client_peer_handle_requests_real(struct http_client_peer *peer)
 {
@@ -263,6 +272,7 @@
 		return;
 	}
 
+	peer->handling_requests = TRUE;
 	t_array_init(&conns_avail, array_count(&peer->conns));
 	do {
 		array_clear(&conns_avail);
@@ -336,9 +346,13 @@
 				"No more requests to service for this peer "
 				"(%u connections exist)", array_count(&peer->conns));
 			http_client_peer_check_idle(peer);
-			return;
+			break;
 		}
 	} while (statistics_dirty);
+	peer->handling_requests = FALSE;
+
+	if (num_pending == 0)
+		return;
 
 	i_assert(idle == 0);
 
@@ -679,13 +693,21 @@
 	http_client_peer_debug(peer, "Lost a connection (%d connections left)",
 		array_count(&peer->conns));
 
+	if (peer->handling_requests) {
+		/* we got here from the request handler loop */
+		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_free(&peer);
+		return;
+	}
+
 	/* if there are pending requests for this peer, create a new connection
 	   for them. */
 	http_client_peer_trigger_request_handler(peer);
-
-	if (array_count(&peer->conns) == 0 &&
-	    http_client_peer_requests_pending(peer, &num_urgent) == 0)
-		http_client_peer_free(&peer);
 }
 
 unsigned int
--- a/src/lib-http/http-client-private.h	Thu Feb 25 01:18:06 2016 +0200
+++ b/src/lib-http/http-client-private.h	Thu Feb 25 22:46:11 2016 +0100
@@ -190,6 +190,7 @@
 	unsigned int seen_100_response:1;/* expect: 100-continue succeeded before */
 	unsigned int allows_pipelining:1;/* peer is known to allow persistent
 	                                     connections */
+	unsigned int handling_requests:1;/* currently running request handler */
 };
 
 struct http_client_queue {