From 79f8a20424633e806447bc9375a5ab403aabc758 Mon Sep 17 00:00:00 2001 From: Stephan Bosch Date: Mon, 8 Feb 2016 22:45:54 +0100 Subject: [PATCH] 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. --- src/lib-http/http-client-connection.c | 53 ++++++++++++++++----------- src/lib-http/http-client-private.h | 2 + src/lib-http/http-client-request.c | 36 +++++++++++++----- 3 files changed, 60 insertions(+), 31 deletions(-) diff --git a/src/lib-http/http-client-connection.c b/src/lib-http/http-client-connection.c index d229730cd2..0947a2a234 100644 --- a/src/lib-http/http-client-connection.c +++ b/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); diff --git a/src/lib-http/http-client-private.h b/src/lib-http/http-client-private.h index ac6b250ab2..5fb3916343 100644 --- a/src/lib-http/http-client-private.h +++ b/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, diff --git a/src/lib-http/http-client-request.c b/src/lib-http/http-client-request.c index e62ac7d7e5..e8f3b64d3d 100644 --- a/src/lib-http/http-client-request.c +++ b/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)