From 8d558a5763725741c77682a4229014222252b4e0 Mon Sep 17 00:00:00 2001 From: Stephan Bosch Date: Wed, 25 May 2016 23:41:47 +0200 Subject: [PATCH] lib-http: client: Fixed bug in handling of lost connections while returning from another ioloop. At one instance the http_client_connection_is_ready() function could have destroyed the connection while the caller still depended on it. Renamed the http_client_connection_is_ready() function to http_client_connection_check_ready(). This now returns -1 when the connection got destroyed. Before it returned a bool that just indicated whether the connection was ready or not. So, there is no need anymore to preserve a connection reference while calling this function. --- src/lib-http/http-client-connection.c | 29 ++++++++++++++------------- src/lib-http/http-client-peer.c | 12 +++++------ src/lib-http/http-client-private.h | 2 +- 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/src/lib-http/http-client-connection.c b/src/lib-http/http-client-connection.c index c8f3565001..880e89a101 100644 --- a/src/lib-http/http-client-connection.c +++ b/src/lib-http/http-client-connection.c @@ -235,7 +235,7 @@ http_client_connection_abort_temp_error(struct http_client_connection **_conn, http_client_connection_close(_conn); } -bool http_client_connection_is_ready(struct http_client_connection *conn) +int http_client_connection_check_ready(struct http_client_connection *conn) { int ret; @@ -245,14 +245,14 @@ bool http_client_connection_is_ready(struct http_client_connection *conn) this way, but theoretically we could, although that would add quite a bit of complexity. */ - return FALSE; + return 0; } if (!conn->connected || conn->output_locked || conn->output_broken || conn->close_indicated || conn->tunneling || http_client_connection_count_pending(conn) >= conn->client->set.max_pipelined_requests) - return FALSE; + return 0; if (conn->last_ioloop != NULL && conn->last_ioloop != current_ioloop) { conn->last_ioloop = current_ioloop; @@ -270,10 +270,10 @@ bool http_client_connection_is_ready(struct http_client_connection *conn) stream_errno != 0 ? i_stream_get_error(conn->conn.input) : "EOF")); - return FALSE; + return -1; } } - return TRUE; + return 1; } static void @@ -408,10 +408,14 @@ int http_client_connection_next_request(struct http_client_connection *conn) struct http_client_request *req = NULL; const char *error; bool pipelined; + int ret; - if (!http_client_connection_is_ready(conn)) { - http_client_connection_debug(conn, "Not ready for next request"); - return 0; + if ((ret=http_client_connection_check_ready(conn)) <= 0) { + if (ret == 0) { + http_client_connection_debug(conn, + "Not ready for next request"); + } + return ret; } /* claim request, but no urgent request can be second in line */ @@ -552,7 +556,6 @@ static void http_client_payload_destroyed(struct http_client_request *req) } conn->incoming_payload = NULL; - http_client_connection_ref(conn); /* input stream may have pending input. make sure input handler gets called (but don't do it directly, since we get get here @@ -565,10 +568,8 @@ static void http_client_payload_destroyed(struct http_client_request *req) } /* room for new requests */ - if (http_client_connection_is_ready(conn)) + if (http_client_connection_check_ready(conn) > 0) http_client_peer_trigger_request_handler(conn->peer); - - http_client_connection_unref(&conn); } static bool @@ -966,7 +967,7 @@ static void http_client_connection_input(struct connection *_conn) conn->peer->allows_pipelining = TRUE; /* room for new requests */ - if (http_client_connection_is_ready(conn)) + if (http_client_connection_check_ready(conn) > 0) http_client_peer_trigger_request_handler(conn->peer); } } @@ -1025,7 +1026,7 @@ int http_client_connection_output(struct http_client_connection *conn) } if (!conn->output_locked) { /* room for new requests */ - if (http_client_connection_is_ready(conn)) + if (http_client_connection_check_ready(conn) > 0) http_client_peer_trigger_request_handler(conn->peer); } } diff --git a/src/lib-http/http-client-peer.c b/src/lib-http/http-client-peer.c index 1876603ed5..d963e03f2b 100644 --- a/src/lib-http/http-client-peer.c +++ b/src/lib-http/http-client-peer.c @@ -268,9 +268,12 @@ http_client_peer_handle_requests_real(struct http_client_peer *peer) /* gather connection statistics */ array_foreach(&peer->conns, conn_idx) { struct http_client_connection *conn = *conn_idx; + int ret; - http_client_connection_ref(conn); - if (http_client_connection_is_ready(conn)) { + if ((ret=http_client_connection_check_ready(conn)) < 0) { + conn_lost = TRUE; + break; + } else if (ret > 0) { struct _conn_available *conn_avail; unsigned int insert_idx, pending_requests; @@ -293,11 +296,6 @@ http_client_peer_handle_requests_real(struct http_client_peer *peer) if (pending_requests == 0) idle++; } - if (!http_client_connection_unref(&conn)) { - conn_lost = TRUE; - break; - } - conn = *conn_idx; /* count the number of connecting and closing connections */ if (conn->closing) closing++; diff --git a/src/lib-http/http-client-private.h b/src/lib-http/http-client-private.h index fd28af20e6..f62630ff43 100644 --- a/src/lib-http/http-client-private.h +++ b/src/lib-http/http-client-private.h @@ -325,7 +325,7 @@ void http_client_connection_stop_request_timeout( struct http_client_connection *conn); unsigned int http_client_connection_count_pending(struct http_client_connection *conn); -bool http_client_connection_is_ready(struct http_client_connection *conn); +int http_client_connection_check_ready(struct http_client_connection *conn); bool http_client_connection_is_idle(struct http_client_connection *conn); int http_client_connection_next_request(struct http_client_connection *conn); void http_client_connection_check_idle(struct http_client_connection *conn); -- 2.47.3