Mercurial > dovecot > core-2.2

changeset 20354:adfa1b7fea65

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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.
author Stephan Bosch <stephan@dovecot.fi>
date Mon, 23 May 2016 02:38:49 +0200
parents e424cc6aa6b9
children 6335d6256a58
files src/lib-http/http-client-connection.c src/lib-http/http-client-peer.c src/lib-http/http-client-private.h
diffstat 3 files changed, 52 insertions(+), 28 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-http/http-client-connection.c	Sun May 22 12:48:37 2016 +0200
+++ b/src/lib-http/http-client-connection.c	Mon May 23 02:38:49 2016 +0200
@@ -46,8 +46,6 @@
 
 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 @@
 	   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_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);
@@ -1414,6 +1415,16 @@
 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;
 
@@ -1447,14 +1458,23 @@
 		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);
 
@@ -1467,6 +1487,13 @@
 
 	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);
 
@@ -1475,17 +1502,6 @@
 	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);
 	return FALSE;
 }
@@ -1501,6 +1517,17 @@
 	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)
--- a/src/lib-http/http-client-peer.c	Sun May 22 12:48:37 2016 +0200
+++ b/src/lib-http/http-client-peer.c	Mon May 23 02:38:49 2016 +0200
@@ -534,7 +534,7 @@
 	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);
 
@@ -746,15 +746,8 @@
 		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);
 }
 
--- a/src/lib-http/http-client-private.h	Sun May 22 12:48:37 2016 +0200
+++ b/src/lib-http/http-client-private.h	Mon May 23 02:38:49 2016 +0200
@@ -159,6 +159,7 @@
 	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 */
@@ -312,6 +313,9 @@
 /* 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);