Skip to content

Commit

Permalink
lib-http: http_client_request_unref() now always sets *req=NULL
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@sirainen
sirainen committed Feb 22, 2016
1 parent 1dead6e commit d1f964d
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 22 deletions.
16 changes: 9 additions & 7 deletions src/lib-http/http-client-connection.c
View file
Expand Up @@ -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))
Expand All @@ -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);
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}

Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
5 changes: 3 additions & 2 deletions src/lib-http/http-client-private.h
View file
Expand Up @@ -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,
Expand All @@ -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);

Expand Down
31 changes: 18 additions & 13 deletions src/lib-http/http-client-request.c
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -198,14 +200,16 @@ 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)
{
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);

Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand All @@ -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,
Expand Down

0 comments on commit d1f964d

Please sign in to comment.