lib-http: client: Make sure that any pending request is aborted and d…

…estroyed 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.

src/lib-http/http-client-connection.c

@@ -121,6 +121,31 @@ http_client_connection_abort_error(struct http_client_connection **_conn,
 	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 @@ http_client_connection_disconnect(struct http_client_connection *conn)
 		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 @@ void http_client_connection_unref(struct http_client_connection **_conn)
 	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 @@ void http_client_connection_unref(struct http_client_connection **_conn)
 
 	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);

src/lib-http/http-client-private.h

@@ -273,6 +273,8 @@ int http_client_init_ssl_ctx(struct http_client *client, const char **error_r);
 
 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,

src/lib-http/http-client-request.c

@@ -166,6 +166,9 @@ void http_client_request_unref(struct http_client_request **_req)
 	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 @@ void http_client_request_unref(struct http_client_request **_req)
 		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 @@ void http_client_request_unref(struct http_client_request **_req)
 	*_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 @@ void http_client_request_error_delayed(struct http_client_request **_req)
 				       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 @@ void http_client_request_error(struct http_client_request *req,
 		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 @@ void http_client_request_abort(struct http_client_request **_req)
 		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)