dovecot-2.2: lib-http: client: Fixed handling of request timeout.

dovecot at dovecot.org dovecot at dovecot.org
Tue Aug 18 20:02:52 UTC 2015


details:   http://hg.dovecot.org/dovecot-2.2/rev/a08e79a3707f
changeset: 18969:a08e79a3707f
user:      Stephan Bosch <stephan at rename-it.nl>
date:      Tue Aug 18 23:02:01 2015 +0300
description:
lib-http: client: Fixed handling of request timeout.
It was inappropriately active when the client needed to take action.
This showed particularly with large payloads sent using
http_client_request_send_payload().

diffstat:

 src/lib-http/http-client-connection.c |  52 ++++++++++++++++++++++++----------
 src/lib-http/http-client-private.h    |   7 ++++
 src/lib-http/http-client-request.c    |  14 ++++++++-
 3 files changed, 55 insertions(+), 18 deletions(-)

diffs (172 lines):

diff -r 04be746494dc -r a08e79a3707f src/lib-http/http-client-connection.c
--- a/src/lib-http/http-client-connection.c	Tue Aug 18 21:10:41 2015 +0300
+++ b/src/lib-http/http-client-connection.c	Tue Aug 18 23:02:01 2015 +0300
@@ -266,6 +266,35 @@
 		msecs/1000, msecs%1000));
 }
 
+void http_client_connection_start_request_timeout(
+	struct http_client_connection *conn)
+{
+	unsigned int timeout_msecs = conn->client->set.request_timeout_msecs;
+
+	if (timeout_msecs == 0)
+		;
+	else if (conn->to_requests != NULL)
+		timeout_reset(conn->to_requests);
+	else {
+		conn->to_requests = timeout_add(timeout_msecs,
+						http_client_connection_request_timeout, conn);
+	}
+}
+
+void http_client_connection_reset_request_timeout(
+	struct http_client_connection *conn)
+{
+	if (conn->to_requests != NULL)
+		timeout_reset(conn->to_requests);
+}
+
+void http_client_connection_stop_request_timeout(
+	struct http_client_connection *conn)
+{
+	if (conn->to_requests != NULL)
+		timeout_remove(&conn->to_requests);
+}
+
 static void
 http_client_connection_continue_timeout(struct http_client_connection *conn)
 {
@@ -316,14 +345,6 @@
 	if (conn->to_idle != NULL)
 		timeout_remove(&conn->to_idle);
 
-	if (conn->client->set.request_timeout_msecs == 0)
-		;
-	else if (conn->to_requests != NULL)
-		timeout_reset(conn->to_requests);
-	else {
-		conn->to_requests = timeout_add(conn->client->set.request_timeout_msecs,
-						http_client_connection_request_timeout, conn);
-	}
 	req->conn = conn;
 	conn->payload_continue = FALSE;
 	if (conn->peer->no_payload_sync)
@@ -489,8 +510,7 @@
 		io_remove(&conn->conn.io);
 		/* we've received the request itself, and we can't reset the
 		   timeout during the payload reading. */
-		if (conn->to_requests != NULL)
-			timeout_remove(&conn->to_requests);
+		http_client_connection_stop_request_timeout(conn);
 	}
 	
 	conn->in_req_callback = TRUE;
@@ -607,8 +627,9 @@
 		http_client_payload_finished(conn);
 		finished++;
 	}
-	if (conn->to_requests != NULL)
-		timeout_reset(conn->to_requests);
+
+	/* we've seen activity from the server; reset request timeout */
+	http_client_connection_reset_request_timeout(conn);
 
 	/* get first waiting request */
 	reqs = array_get(&conn->request_wait_list, &count);
@@ -786,8 +807,7 @@
 			payload_type = http_client_request_get_payload_type(req);
 		} else {
 			/* no more requests waiting for the connection */
-			if (conn->to_requests != NULL)
-				timeout_remove(&conn->to_requests);
+			http_client_connection_stop_request_timeout(conn);
 			req = NULL;
 			payload_type = HTTP_RESPONSE_PAYLOAD_TYPE_ALLOWED;
 		}
@@ -839,8 +859,8 @@
 	const char *error;
 	int ret;
 
-	if (conn->to_requests != NULL)
-		timeout_reset(conn->to_requests);
+	/* we've seen activity from the server; reset request timeout */
+	http_client_connection_reset_request_timeout(conn);
 
 	if ((ret = o_stream_flush(output)) <= 0) {
 		if (ret < 0) {
diff -r 04be746494dc -r a08e79a3707f src/lib-http/http-client-private.h
--- a/src/lib-http/http-client-private.h	Tue Aug 18 21:10:41 2015 +0300
+++ b/src/lib-http/http-client-private.h	Tue Aug 18 23:02:01 2015 +0300
@@ -291,6 +291,12 @@
 void http_client_connection_unref(struct http_client_connection **_conn);
 void http_client_connection_close(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);
+void http_client_connection_reset_request_timeout(
+	struct http_client_connection *conn);
+void http_client_connection_stop_request_timeout(
+	struct http_client_connection *conn);
 unsigned int
 http_client_connection_count_pending(struct http_client_connection *conn);
 bool http_client_connection_is_ready(struct http_client_connection *conn);
@@ -301,6 +307,7 @@
 void http_client_connection_start_tunnel(struct http_client_connection **_conn,
 	struct http_client_tunnel *tunnel);
 
+
 unsigned int http_client_peer_addr_hash
 	(const struct http_client_peer_addr *peer) ATTR_PURE;
 int http_client_peer_addr_cmp
diff -r 04be746494dc -r a08e79a3707f src/lib-http/http-client-request.c
--- a/src/lib-http/http-client-request.c	Tue Aug 18 21:10:41 2015 +0300
+++ b/src/lib-http/http-client-request.c	Tue Aug 18 23:02:01 2015 +0300
@@ -536,7 +536,10 @@
 	/* advance state only when request didn't get aborted in the mean time */
 	if (req->state != HTTP_REQUEST_STATE_ABORTED) {
 		i_assert(req->state == HTTP_REQUEST_STATE_PAYLOAD_OUT);
+
+		/* we're now waiting for a response from the server */
 		req->state = HTTP_REQUEST_STATE_WAITING;
+		http_client_connection_start_request_timeout(req->conn);
 	}
 
 	/* release connection */
@@ -703,6 +706,7 @@
 	i_assert(ret >= 0);
 
 	if (i_stream_is_eof(req->payload_input)) {
+		/* finished sending */
 		if (!req->payload_chunked &&
 		    req->payload_input->v_offset - req->payload_offset != req->payload_size) {
 			*error_r = t_strdup_printf("BUG: stream '%s' input size changed: "
@@ -714,20 +718,26 @@
 		}
 
 		if (req->payload_wait) {
+			/* this chunk of input is finished
+			   (client needs to act; disable timeout) */
 			conn->output_locked = TRUE;
+			http_client_connection_stop_request_timeout(conn);
 			if (req->client->ioloop != NULL)
 				io_loop_stop(req->client->ioloop);
 		} else {
+			/* finished sending payload */
 			http_client_request_finish_payload_out(req);
 		}
 	} else if (i_stream_get_data_size(req->payload_input) > 0) {
-		/* output is blocking */
+		/* output is blocking (server needs to act; enable timeout) */
 		conn->output_locked = TRUE;
+		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 */
+		/* input is blocking (client needs to act; disable timeout) */
 		conn->output_locked = TRUE;	
+		http_client_connection_stop_request_timeout(conn);
 		conn->io_req_payload = io_add_istream(req->payload_input,
 			http_client_request_payload_input, req);
 	}


More information about the dovecot-cvs mailing list