Mercurial > dovecot > core-2.2
changeset 21263:1c92017ddd88
lib-http: client: Fixed assert failure occurring when a new connection fails for a peer that has active connections.
Fixes: Panic: file http-client-queue.c: line 481 (http_client_queue_connection_failure): assertion failed: (queue->cur_peer == NULL)
author | Stephan Bosch <stephan.bosch@dovecot.fi> |
---|---|
date | Mon, 21 Nov 2016 23:19:26 +0100 |
parents | 0deeb9440b67 |
children | 8f33680c6722 |
files | src/lib-http/http-client-queue.c src/lib-http/test-http-client-errors.c |
diffstat | 2 files changed, 144 insertions(+), 2 deletions(-) [+] |
line wrap: on
line diff
--- a/src/lib-http/http-client-queue.c Fri Dec 02 22:42:24 2016 +0200 +++ b/src/lib-http/http-client-queue.c Mon Nov 21 23:19:26 2016 +0100 @@ -478,8 +478,17 @@ struct http_client_peer *failed_peer; struct http_client_peer *const *peer_idx; - i_assert(queue->cur_peer == NULL); - i_assert(array_count(&queue->pending_peers) > 0); + if (queue->cur_peer != NULL) { + /* The peer still has some working connections, which means that + pending requests wait until they're picked up by those connections + or the remaining connections fail as well. In the latter case, + connecting to different peer can resolve the situation, but only + if there is more than one IP. In any other case, the requests will + eventually fail. In the future we could start connections to the next + IP at this point already, but that is no small change. */ + i_assert(array_count(&queue->pending_peers) == 0); + return; + } http_client_queue_debug(queue, "Failed to set up connection to %s%s: %s "
--- a/src/lib-http/test-http-client-errors.c Fri Dec 02 22:42:24 2016 +0200 +++ b/src/lib-http/test-http-client-errors.c Mon Nov 21 23:19:26 2016 +0100 @@ -2193,6 +2193,138 @@ } /* + * Peer reuse failure + */ + +/* server */ + +static void +test_peer_reuse_failure_input(struct server_connection *conn) +{ + static unsigned int seq = 0; + static const char *resp = + "HTTP/1.1 200 OK\r\n" + "\r\n"; + o_stream_nsend_str(conn->conn.output, resp); + if (seq++ > 2) { + server_connection_deinit(&conn); + io_loop_stop(current_ioloop); + } +} + +static void test_server_peer_reuse_failure(unsigned int index) +{ + test_server_input = test_peer_reuse_failure_input; + test_server_run(index); +} + +/* client */ + +struct _peer_reuse_failure { + struct timeout *to; + bool first:1; +}; + +static void +test_client_peer_reuse_failure_response2( + const struct http_response *resp, + struct _peer_reuse_failure *ctx) +{ + if (debug) + i_debug("RESPONSE: %u %s", resp->status, resp->reason); + + test_assert(resp->status == HTTP_CLIENT_REQUEST_ERROR_CONNECT_FAILED); + test_assert(resp->reason != NULL && *resp->reason != '\0'); + i_free(ctx); + io_loop_stop(ioloop); +} + +static void +test_client_peer_reuse_failure_next(struct _peer_reuse_failure *ctx) +{ + struct http_client_request *hreq; + + timeout_remove(&ctx->to); + + hreq = http_client_request(http_client, + "GET", net_ip2addr(&bind_ip), "/peer-reuse-next.txt", + test_client_peer_reuse_failure_response2, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_submit(hreq); +} + +static void +test_client_peer_reuse_failure_response1( + const struct http_response *resp, + struct _peer_reuse_failure *ctx) +{ + if (debug) + i_debug("RESPONSE: %u %s", resp->status, resp->reason); + + if (ctx->first) { + test_assert(resp->status == 200); + + ctx->first = FALSE; + ctx->to = timeout_add_short(500, test_client_peer_reuse_failure_next, ctx); + } else { + test_assert(resp->status == HTTP_CLIENT_REQUEST_ERROR_CONNECT_FAILED); + } + + test_assert(resp->reason != NULL && *resp->reason != '\0'); +} + +static bool +test_client_peer_reuse_failure(const struct http_client_settings *client_set) +{ + struct http_client_request *hreq; + struct _peer_reuse_failure *ctx; + + ctx = i_new(struct _peer_reuse_failure, 1); + ctx->first = TRUE; + + http_client = http_client_init(client_set); + + hreq = http_client_request(http_client, + "GET", net_ip2addr(&bind_ip), "/peer-reuse.txt", + test_client_peer_reuse_failure_response1, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_submit(hreq); + + hreq = http_client_request(http_client, + "GET", net_ip2addr(&bind_ip), "/peer-reuse.txt", + test_client_peer_reuse_failure_response1, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_submit(hreq); + + hreq = http_client_request(http_client, + "GET", net_ip2addr(&bind_ip), "/peer-reuse.txt", + test_client_peer_reuse_failure_response1, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_submit(hreq); + + return TRUE; +} + +/* test */ + +static void test_peer_reuse_failure(void) +{ + struct http_client_settings http_client_set; + + test_client_defaults(&http_client_set); + http_client_set.max_connect_attempts = 1; + http_client_set.max_idle_time_msecs = 500; + + test_begin("peer reuse failure"); + test_run_client_server(&http_client_set, + test_client_peer_reuse_failure, + test_server_peer_reuse_failure, 1, + NULL); + test_end(); +} + + +/* * All tests */ @@ -2218,6 +2350,7 @@ test_dns_timeout, test_dns_lookup_failure, test_dns_lookup_ttl, + test_peer_reuse_failure, NULL };