Mercurial > dovecot > core-2.2

changeset 19726:f40f5e721744

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
lib-http: client: Make sure that any pending request is aborted and destroyed before connection FDs are closed. This way, any payload io struct created from the response callback can be freed before the associated FD becomes invalid. This would cause an assert failure otherwise.
author Stephan Bosch <stephan@rename-it.nl>
date Mon, 08 Feb 2016 22:45:54 +0100
parents ab6f3658c5eb
children 8f72e4f0ed5d
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, 60 insertions(+), 31 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-http/http-client-connection.c	Mon Feb 08 22:44:52 2016 +0100
+++ b/src/lib-http/http-client-connection.c	Mon Feb 08 22:45:54 2016 +0100
@@ -121,6 +121,31 @@
 	http_client_connection_close(_conn);
 }
 
+static void
+http_client_connection_abort_any_requests(struct http_client_connection *conn)
+{
+	struct http_client_request **req;
+
+	if (array_is_created(&conn->request_wait_list)) {
+		array_foreach_modifiable(&conn->request_wait_list, req) {
+			i_assert((*req)->submitted);
+			http_client_request_error(*req,
+				HTTP_CLIENT_REQUEST_ERROR_ABORTED,
+				"Aborting");
+			http_client_request_unref(req);
+		}
+		array_clear(&conn->request_wait_list);
+	}
+	if (conn->pending_request != NULL) {
+		struct http_client_request *pending_req = conn->pending_request;
+		conn->pending_request = NULL;
+		http_client_request_error(pending_req,
+			HTTP_CLIENT_REQUEST_ERROR_ABORTED,
+			"Aborting");
+		http_client_request_unref(&pending_req);
+	}
+}
+
 static const char *
 http_client_connection_get_timing_info(struct http_client_connection *conn)
 {
@@ -1346,6 +1371,11 @@
 		conn->incoming_payload = NULL;
 	}
 
+	http_client_connection_abort_any_requests(conn);
+
+	if (conn->http_parser != NULL)
+		http_response_parser_deinit(&conn->http_parser);
+
 	if (conn->connect_initialized)
 		connection_disconnect(&conn->conn);
 
@@ -1369,7 +1399,6 @@
 	struct http_client_connection *const *conn_idx;
 	ARRAY_TYPE(http_client_connection) *conn_arr;
 	struct http_client_peer *peer = conn->peer;
-	struct http_client_request **req;
 
 	i_assert(conn->refcount > 0);
 
@@ -1380,28 +1409,8 @@
 
 	http_client_connection_disconnect(conn);
 
-	/* abort all pending requests (not supposed to happen here) */
-	if (array_is_created(&conn->request_wait_list)) {
-		array_foreach_modifiable(&conn->request_wait_list, req) {
-			i_assert((*req)->submitted);
-			http_client_request_error(*req,
-				HTTP_CLIENT_REQUEST_ERROR_ABORTED,
-				"Aborting");
-			http_client_request_unref(req);
-		}
+	if (array_is_created(&conn->request_wait_list))
 		array_free(&conn->request_wait_list);
-	}
-	if (conn->pending_request != NULL) {
-		struct http_client_request *pending_req = conn->pending_request;
-		conn->pending_request = NULL;
-		http_client_request_error(pending_req,
- 			HTTP_CLIENT_REQUEST_ERROR_ABORTED,
-			"Aborting");
-		http_client_request_unref(&pending_req);
-	}
-
-	if (conn->http_parser != NULL)
-		http_response_parser_deinit(&conn->http_parser);
 
 	if (conn->ssl_iostream != NULL)
 		ssl_iostream_unref(&conn->ssl_iostream);
--- a/src/lib-http/http-client-private.h	Mon Feb 08 22:44:52 2016 +0100
+++ b/src/lib-http/http-client-private.h	Mon Feb 08 22:45:54 2016 +0100
@@ -273,6 +273,8 @@
 
 void http_client_request_ref(struct http_client_request *req);
 void http_client_request_unref(struct http_client_request **_req);
+void http_client_request_destroy(struct http_client_request **_req);
+
 int http_client_request_delay_from_response(struct http_client_request *req,
 	const struct http_response *response);
 void http_client_request_get_peer_addr(const struct http_client_request *req,
--- a/src/lib-http/http-client-request.c	Mon Feb 08 22:44:52 2016 +0100
+++ b/src/lib-http/http-client-request.c	Mon Feb 08 22:45:54 2016 +0100
@@ -166,6 +166,9 @@
 	if (--req->refcount > 0)
 		return;
 
+	http_client_request_debug(req, "Free (requests left=%d)",
+		client->requests_count);
+
 	/* cannot be destroyed while it is still pending */
 	i_assert(req->conn == NULL || req->conn->pending_request == NULL);
 
@@ -183,12 +186,6 @@
 		client->requests_count--;
 	}
 
-	http_client_request_debug(req, "Destroy (requests left=%d)",
-		client->requests_count);
-
-	if (req->queue != NULL)
-		http_client_queue_drop_request(req->queue, req);
-
 	if (client->requests_count == 0 && client->ioloop != NULL)
 		io_loop_stop(client->ioloop);
 
@@ -204,6 +201,27 @@
 	*_req = NULL;
 }
 
+void http_client_request_destroy(struct http_client_request **_req)
+{
+	struct http_client_request *req = *_req;
+	struct http_client *client = req->client;
+
+	http_client_request_debug(req, "Destroy (requests left=%d)",
+		client->requests_count);
+
+	if (req->queue != NULL)
+		http_client_queue_drop_request(req->queue, req);
+
+	if (req->destroy_callback != NULL) {
+		void (*callback)(void *) = req->destroy_callback;
+
+		req->destroy_callback = NULL;
+		callback(req->destroy_context);
+	}
+
+	http_client_request_unref(_req);
+}
+
 void http_client_request_set_port(struct http_client_request *req,
 	in_port_t port)
 {
@@ -1009,7 +1027,7 @@
 				       req->delayed_error);
 	if (req->queue != NULL)
 		http_client_queue_drop_request(req->queue, req);
-	http_client_request_unref(_req);
+	http_client_request_destroy(_req);
 }
 
 void http_client_request_error(struct http_client_request *req,
@@ -1033,7 +1051,7 @@
 		http_client_delay_request_error(req->client, req);
 	} else {
 		http_client_request_send_error(req, status, error);
-		http_client_request_unref(&req);
+		http_client_request_destroy(&req);
 	}
 }
 
@@ -1056,7 +1074,7 @@
 		http_client_queue_drop_request(req->queue, req);
 	if (req->payload_wait && req->client->ioloop != NULL)
 		io_loop_stop(req->client->ioloop);
-	http_client_request_unref(_req);
+	http_client_request_destroy(_req);
 }
 
 void http_client_request_finish(struct http_client_request **_req)