From: Timo Sirainen Date: Mon, 22 Feb 2016 18:34:46 +0000 (+0200) Subject: lib-http: http_client_request_unref() now always sets *req=NULL X-Git-Tag: 2.2.22.rc1~103 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d1f964d3f1dd9c5868b134c4f44dd63f3722eef7;p=thirdparty%2Fdovecot%2Fcore.git lib-http: http_client_request_unref() now always sets *req=NULL This makes its behavior consistent with other APIs in Dovecot. Also http_client_request_finish() no longer sets req=NULL, because all of its callers already keep a reference. Instead added an assert to make sure the reference is there. --- diff --git a/src/lib-http/http-client-connection.c b/src/lib-http/http-client-connection.c index 85fe67e789..43c6d1f31c 100644 --- a/src/lib-http/http-client-connection.c +++ b/src/lib-http/http-client-connection.c @@ -518,7 +518,7 @@ static void http_client_payload_destroyed(struct http_client_request *req) conn->incoming_payload = NULL; conn->pending_request = NULL; http_client_connection_ref(conn); - http_client_request_finish(&req); + http_client_request_finish(req); /* room for new requests */ if (http_client_connection_is_ready(conn)) @@ -532,7 +532,6 @@ static void http_client_payload_destroyed(struct http_client_request *req) conn->to_input = timeout_add_short(0, http_client_payload_destroyed_timeout, conn); - i_assert(req != NULL); http_client_request_unref(&req); http_client_connection_unref(&conn); } @@ -575,7 +574,7 @@ http_client_connection_return_response(struct http_client_request *req, if (!http_client_connection_unref(&req->conn)) { /* the callback managed to get this connection destroyed */ if (!retrying) - http_client_request_finish(&req); + http_client_request_finish(req); http_client_request_unref(&req); return FALSE; } @@ -611,7 +610,7 @@ http_client_connection_return_response(struct http_client_request *req, } } else { req->conn = NULL; - http_client_request_finish(&req); + http_client_request_finish(req); http_client_request_unref(&req); } @@ -632,7 +631,7 @@ static void http_client_connection_input(struct connection *_conn) (struct http_client_connection *)_conn; struct http_response response; struct http_client_request *const *reqs; - struct http_client_request *req = NULL; + struct http_client_request *req = NULL, *req_ref; enum http_response_payload_type payload_type; unsigned int count; int finished = 0, ret; @@ -799,8 +798,11 @@ static void http_client_connection_input(struct connection *_conn) /* remove request from queue */ array_delete(&conn->request_wait_list, 0, 1); aborted = (req->state == HTTP_REQUEST_STATE_ABORTED); - i_assert(req->refcount > 1 || aborted); - http_client_request_unref(&req); + req_ref = req; + if (!http_client_request_unref(&req_ref)) { + i_assert(aborted); + req = NULL; + } conn->close_indicated = response.connection_close; diff --git a/src/lib-http/http-client-private.h b/src/lib-http/http-client-private.h index 474616c101..f78011f6fa 100644 --- a/src/lib-http/http-client-private.h +++ b/src/lib-http/http-client-private.h @@ -271,7 +271,8 @@ struct http_client { 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); +/* Returns FALSE if unrefing destroyed the request entirely */ +bool 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, @@ -297,7 +298,7 @@ void http_client_request_error(struct http_client_request *req, unsigned int status, const char *error); void http_client_request_redirect(struct http_client_request *req, unsigned int status, const char *location); -void http_client_request_finish(struct http_client_request **_req); +void http_client_request_finish(struct http_client_request *req); struct connection_list *http_client_connection_list_init(void); diff --git a/src/lib-http/http-client-request.c b/src/lib-http/http-client-request.c index c1bc49e532..2c985d7d13 100644 --- a/src/lib-http/http-client-request.c +++ b/src/lib-http/http-client-request.c @@ -156,15 +156,17 @@ void http_client_request_ref(struct http_client_request *req) req->refcount++; } -void http_client_request_unref(struct http_client_request **_req) +bool http_client_request_unref(struct http_client_request **_req) { struct http_client_request *req = *_req; struct http_client *client = req->client; i_assert(req->refcount > 0); + *_req = NULL; + if (--req->refcount > 0) - return; + return TRUE; http_client_request_debug(req, "Free (requests left=%d)", client->requests_count); @@ -198,7 +200,7 @@ void http_client_request_unref(struct http_client_request **_req) if (req->headers != NULL) str_free(&req->headers); pool_unref(&req->pool); - *_req = NULL; + return FALSE; } void http_client_request_destroy(struct http_client_request **_req) @@ -206,6 +208,8 @@ void http_client_request_destroy(struct http_client_request **_req) struct http_client_request *req = *_req; struct http_client *client = req->client; + *_req = NULL; + http_client_request_debug(req, "Destroy (requests left=%d)", client->requests_count); @@ -218,8 +222,7 @@ void http_client_request_destroy(struct http_client_request **_req) req->destroy_callback = NULL; callback(req->destroy_context); } - - http_client_request_unref(_req); + http_client_request_unref(&req); } void http_client_request_set_port(struct http_client_request *req, @@ -695,8 +698,7 @@ http_client_request_continue_payload(struct http_client_request **_req, /* callback may have messed with our pointer, so unref using local variable */ - http_client_request_unref(&req); - if (req == NULL) + if (!http_client_request_unref(&req)) *_req = NULL; if (conn != NULL) @@ -1022,12 +1024,14 @@ void http_client_request_error_delayed(struct http_client_request **_req) i_assert(req->state == HTTP_REQUEST_STATE_ABORTED); + *_req = NULL; + i_assert(req->delayed_error != NULL && req->delayed_error_status != 0); http_client_request_send_error(req, req->delayed_error_status, req->delayed_error); if (req->queue != NULL) http_client_queue_drop_request(req->queue, req); - http_client_request_destroy(_req); + http_client_request_destroy(&req); } void http_client_request_error(struct http_client_request *req, @@ -1060,6 +1064,8 @@ void http_client_request_abort(struct http_client_request **_req) struct http_client_request *req = *_req; bool sending = (req->state == HTTP_REQUEST_STATE_PAYLOAD_OUT); + *_req = NULL; + if (req->state >= HTTP_REQUEST_STATE_FINISHED) return; @@ -1074,16 +1080,15 @@ 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_destroy(_req); + http_client_request_destroy(&req); } -void http_client_request_finish(struct http_client_request **_req) +void http_client_request_finish(struct http_client_request *req) { - struct http_client_request *req = *_req; - if (req->state >= HTTP_REQUEST_STATE_FINISHED) return; + i_assert(req->refcount > 1); http_client_request_debug(req, "Finished"); req->callback = NULL; @@ -1093,7 +1098,7 @@ void http_client_request_finish(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_unref(&req); } void http_client_request_redirect(struct http_client_request *req,