Mercurial > dovecot > core-2.2

changeset 19961:3f6a74b9ce54

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
lib-http: client: Fixed request timeout handling during pipelining. The timeout was not managed correctly. If an earlier request finished, it would not restart the timeout for the next pending request. Also, filling the pipeline caused the timout to be reset inappropriately, postponing its expiry.
author Stephan Bosch <stephan@rename-it.nl>
date Fri, 25 Mar 2016 02:48:55 +0900
parents 5f32b22684ee
children 81903a4b2e98
files src/lib-http/http-client-connection.c src/lib-http/http-client-private.h src/lib-http/http-client-request.c
diffstat 3 files changed, 33 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-http/http-client-connection.c	Fri Mar 25 02:47:28 2016 +0900
+++ b/src/lib-http/http-client-connection.c	Fri Mar 25 02:48:55 2016 +0900
@@ -353,10 +353,13 @@
 static void
 http_client_connection_continue_timeout(struct http_client_connection *conn)
 {
-	struct http_client_request *const *req_idx;
+	struct http_client_request *const *wait_reqs;
 	struct http_client_request *req;
+	unsigned int wait_count;
 	const char *error;
 
+	i_assert(conn->pending_request == NULL);
+
 	if (conn->to_response != NULL)
 		timeout_remove(&conn->to_response);
 	conn->peer->no_payload_sync = TRUE;
@@ -364,13 +367,12 @@
 	http_client_connection_debug(conn, 
 		"Expected 100-continue response timed out; sending payload anyway");
 
-	i_assert(array_count(&conn->request_wait_list) > 0);
-	req_idx = array_idx(&conn->request_wait_list,
-		array_count(&conn->request_wait_list)-1);
-	req = req_idx[0];
+	wait_reqs = array_get(&conn->request_wait_list, &wait_count);
+	i_assert(wait_count == 1);
+	req = wait_reqs[wait_count-1];
 
 	req->payload_sync_continue = TRUE;
-	if (http_client_request_send_more(req, &error) < 0) {
+	if (http_client_request_send_more(req, FALSE, &error) < 0) {
 		http_client_connection_abort_temp_error(&conn,
 			HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
 			t_strdup_printf("Failed to send request: %s", error));
@@ -381,7 +383,7 @@
 {
 	struct http_client_request *req = NULL;
 	const char *error;
-	bool have_pending_requests;
+	bool pipelined;
 
 	if (!http_client_connection_is_ready(conn)) {
 		http_client_connection_debug(conn, "Not ready for next request");
@@ -389,9 +391,9 @@
 	}
 
 	/* claim request, but no urgent request can be second in line */
-	have_pending_requests = array_count(&conn->request_wait_list) > 0 ||
+	pipelined = array_count(&conn->request_wait_list) > 0 ||
 		conn->pending_request != NULL;
-	req = http_client_peer_claim_request(conn->peer, have_pending_requests);
+	req = http_client_peer_claim_request(conn->peer, pipelined);
 	if (req == NULL)
 		return 0;	
 
@@ -411,7 +413,7 @@
 	http_client_connection_debug(conn, "Claimed request %s",
 		http_client_request_label(req));
 
-	if (http_client_request_send(req, &error) < 0) {
+	if (http_client_request_send(req, pipelined, &error) < 0) {
 		http_client_connection_abort_temp_error(&conn,
 			HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
 			t_strdup_printf("Failed to send request: %s", error));
@@ -431,6 +433,7 @@
 		    indefinite period before sending the message body.
 	 */
 	if (req->payload_sync && !conn->peer->seen_100_response) {
+		i_assert(!pipelined);
 		i_assert(req->payload_chunked || req->payload_size > 0);
 		i_assert(conn->to_response == NULL);
 		conn->to_response =	timeout_add(HTTP_CLIENT_CONTINUE_TIMEOUT_MSECS,
@@ -485,6 +488,8 @@
 	timeout_remove(&conn->to_input);
 	conn->conn.io = io_add_istream(conn->conn.input,
 				       http_client_connection_input, &conn->conn);
+	if (array_count(&conn->request_wait_list) > 0)
+		http_client_connection_start_request_timeout(conn);
 }
 
 static void
@@ -762,7 +767,7 @@
 				return;
 			}
 
-			if (http_client_request_send_more(req, &error) < 0) {
+			if (http_client_request_send_more(req, FALSE, &error) < 0) {
 				http_client_connection_abort_temp_error(&conn,
 					HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
 					t_strdup_printf("Failed to send request: %s", error));
@@ -889,7 +894,6 @@
 			payload_type = http_client_request_get_payload_type(req);
 		} else {
 			/* no more requests waiting for the connection */
-			http_client_connection_stop_request_timeout(conn);
 			req = NULL;
 			payload_type = HTTP_RESPONSE_PAYLOAD_TYPE_ALLOWED;
 		}
@@ -964,6 +968,7 @@
 	reqs = array_get(&conn->request_wait_list, &count);
 	if (count > 0 && conn->output_locked) {
 		struct http_client_request *req = reqs[count-1];
+		bool pipelined = (count > 1 || conn->pending_request != NULL);
 
 		if (req->state == HTTP_REQUEST_STATE_ABORTED) {
 			http_client_connection_debug(conn,
@@ -978,7 +983,7 @@
 		}
 
 		if (!req->payload_sync || req->payload_sync_continue) {
-			if (http_client_request_send_more(req, &error) < 0) {
+			if (http_client_request_send_more(req, pipelined, &error) < 0) {
 				http_client_connection_abort_temp_error(&conn,
 					HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
 					t_strdup_printf("Connection lost: %s", error));
--- a/src/lib-http/http-client-private.h	Fri Mar 25 02:47:28 2016 +0900
+++ b/src/lib-http/http-client-private.h	Fri Mar 25 02:48:55 2016 +0900
@@ -282,9 +282,9 @@
 enum http_response_payload_type
 http_client_request_get_payload_type(struct http_client_request *req);
 int http_client_request_send(struct http_client_request *req,
-			    const char **error_r);
+			    bool pipelined, const char **error_r);
 int http_client_request_send_more(struct http_client_request *req,
-				  const char **error_r);
+				  bool pipelined, const char **error_r);
 bool http_client_request_callback(struct http_client_request *req,
 	struct http_response *response);
 void http_client_request_connect_callback(struct http_client_request *req,
--- a/src/lib-http/http-client-request.c	Fri Mar 25 02:47:28 2016 +0900
+++ b/src/lib-http/http-client-request.c	Fri Mar 25 02:48:55 2016 +0900
@@ -748,7 +748,7 @@
 }
 
 int http_client_request_send_more(struct http_client_request *req,
-				  const char **error_r)
+				  bool pipelined, const char **error_r)
 {
 	struct http_client_connection *conn = req->conn;
 	struct ostream *output = req->payload_output;
@@ -804,6 +804,7 @@
 		if (req->payload_wait) {
 			/* this chunk of input is finished
 			   (client needs to act; disable timeout) */
+			i_assert(!pipelined);
 			conn->output_locked = TRUE;
 			http_client_connection_stop_request_timeout(conn);
 			if (req->client->ioloop != NULL)
@@ -815,13 +816,15 @@
 	} else if (i_stream_get_data_size(req->payload_input) > 0) {
 		/* output is blocking (server needs to act; enable timeout) */
 		conn->output_locked = TRUE;
-		http_client_connection_start_request_timeout(conn);
+		if (!pipelined)
+			http_client_connection_start_request_timeout(conn);
 		o_stream_set_flush_pending(output, TRUE);
 		http_client_request_debug(req, "Partially sent payload");
 	} else {
 		/* input is blocking (client needs to act; disable timeout) */
 		conn->output_locked = TRUE;	
-		http_client_connection_stop_request_timeout(conn);
+		if (!pipelined)
+			http_client_connection_stop_request_timeout(conn);
 		conn->io_req_payload = io_add_istream(req->payload_input,
 			http_client_request_payload_input, req);
 	}
@@ -829,7 +832,7 @@
 }
 
 static int http_client_request_send_real(struct http_client_request *req,
-					 const char **error_r)
+					 bool pipelined, const char **error_r)
 {
 	const struct http_client_settings *set = &req->client->set;
 	struct http_client_connection *conn = req->conn;
@@ -946,7 +949,8 @@
 
 		if (req->payload_output != NULL) {
 			if (!req->payload_sync) {
-				if (http_client_request_send_more(req, error_r) < 0)
+				if (http_client_request_send_more
+					(req, pipelined, error_r) < 0)
 					ret = -1;
 			} else {
 				http_client_request_debug(req, "Waiting for 100-continue");
@@ -954,7 +958,8 @@
 			}
 		} else {
 			req->state = HTTP_REQUEST_STATE_WAITING;
-			http_client_connection_start_request_timeout(req->conn);
+			if (!pipelined)
+				http_client_connection_start_request_timeout(req->conn);
 			conn->output_locked = FALSE;
 		}
 		if (ret >= 0 && o_stream_flush(output) < 0) {
@@ -969,13 +974,13 @@
 }
 
 int http_client_request_send(struct http_client_request *req,
-			     const char **error_r)
+			     bool pipelined, const char **error_r)
 {
 	char *errstr = NULL;
 	int ret;
 
 	T_BEGIN {
-		ret = http_client_request_send_real(req, error_r);
+		ret = http_client_request_send_real(req, pipelined, error_r);
 		if (ret < 0)
 			errstr = i_strdup(*error_r);
 	} T_END;