changeset 19817:fa23955f6028

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.
author Timo Sirainen <timo.sirainen@dovecot.fi>
date Mon, 22 Feb 2016 20:34:46 +0200
parents e2f9c117aef7
children 2cc7c59f1311
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, 30 insertions(+), 22 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-http/http-client-connection.c	Mon Feb 22 20:14:57 2016 +0200
+++ b/src/lib-http/http-client-connection.c	Mon Feb 22 20:34:46 2016 +0200
@@ -518,7 +518,7 @@
 	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 @@
 	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 @@
 	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 @@
 		}
 	} else {
 		req->conn = NULL;
-		http_client_request_finish(&req);
+		http_client_request_finish(req);
 		http_client_request_unref(&req);
 	}
 
@@ -632,7 +631,7 @@
 		(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 @@
 		/* 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;
 
--- a/src/lib-http/http-client-private.h	Mon Feb 22 20:14:57 2016 +0200
+++ b/src/lib-http/http-client-private.h	Mon Feb 22 20:34:46 2016 +0200
@@ -271,7 +271,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);
+/* 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 @@
 	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);
 
--- a/src/lib-http/http-client-request.c	Mon Feb 22 20:14:57 2016 +0200
+++ b/src/lib-http/http-client-request.c	Mon Feb 22 20:34:46 2016 +0200
@@ -156,15 +156,17 @@
 	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 @@
 	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 @@
 	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 @@
 		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 @@
 
 	/* 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 @@
 
 	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 @@
 	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 @@
 		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 @@
 		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,