From: Azat Khuzhin Date: Mon, 4 Mar 2019 03:53:42 +0000 (+0300) Subject: http: implement separate timeouts for read/write/connect phase X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5ee507c889b019e9296f48e4f133c439e3040c9e;p=libevent http: implement separate timeouts for read/write/connect phase This patch allows to change timeout for next events read/write/connect separatelly, using new API: - client: evhttp_connection_set_connect_timeout_tv() -- for connect evhttp_connection_set_read_timeout_tv() -- for read evhttp_connection_set_write_timeout_tv() -- for write - server: evhttp_set_read_timeout_tv() -- for read evhttp_set_write_timeout_tv() -- for write It also changes a logic a little, before there was next fallbacks which does not handled in new API: - HTTP_CONNECT_TIMEOUT - HTTP_WRITE_TIMEOUT - HTTP_READ_TIMEOUT And introduce another internal flag (EVHTTP_CON_TIMEOUT_ADJUSTED) that will be used in evrpc, which adjust evhttp_connection timeout only if it is not default. Fixes: #692 Fixes: #715 --- diff --git a/evrpc.c b/evrpc.c index 68fa1b90..3b5260fc 100644 --- a/evrpc.c +++ b/evrpc.c @@ -593,7 +593,7 @@ evrpc_pool_add_connection(struct evrpc_pool *pool, * unless a timeout was specifically set for a connection, * the connection inherits the timeout from the pool. */ - if (!evutil_timerisset(&connection->timeout)) + if (!(connection->flags & EVHTTP_CON_TIMEOUT_ADJUSTED)) evhttp_connection_set_timeout(connection, pool->timeout); /* diff --git a/http-internal.h b/http-internal.h index 9e5b0f95..fb39a650 100644 --- a/http-internal.h +++ b/http-internal.h @@ -17,6 +17,7 @@ #define HTTP_CONNECT_TIMEOUT 45 #define HTTP_WRITE_TIMEOUT 50 #define HTTP_READ_TIMEOUT 50 +#define HTTP_INITIAL_RETRY_TIMEOUT 2 #define HTTP_PREFIX "http://" #define HTTP_DEFAULTPORT 80 @@ -79,8 +80,13 @@ struct evhttp_connection { /* Installed when attempt to read HTTP error after write failed, see * EVHTTP_CON_READ_ON_WRITE_ERROR */ #define EVHTTP_CON_READING_ERROR (EVHTTP_CON_AUTOFREE << 1) +/* Timeout is not default */ +#define EVHTTP_CON_TIMEOUT_ADJUSTED (EVHTTP_CON_READING_ERROR << 1) + + struct timeval timeout_connect; /* timeout for connect phase */ + struct timeval timeout_read; /* timeout for read */ + struct timeval timeout_write; /* timeout for write */ - struct timeval timeout; /* timeout for events */ int retry_cnt; /* retry count */ int retry_max; /* maximum number of retries */ struct timeval initial_retry_timeout; /* Timeout for low long to wait @@ -153,7 +159,8 @@ struct evhttp { /* NULL if this server is not a vhost */ char *vhost_pattern; - struct timeval timeout; + struct timeval timeout_read; /* timeout for read */ + struct timeval timeout_write; /* timeout for write */ size_t default_max_headers_size; ev_uint64_t default_max_body_size; diff --git a/http.c b/http.c index 3b48a738..1768781f 100644 --- a/http.c +++ b/http.c @@ -715,6 +715,38 @@ evhttp_request_free_(struct evhttp_connection *evcon, struct evhttp_request *req evhttp_request_free_auto(req); } +static void +evhttp_set_timeout_tv_(struct timeval *tv, const struct timeval *timeout, int def) +{ + if (timeout == NULL && def != -1) { + tv->tv_sec = def; + tv->tv_usec = 0; + return; + } + + if (timeout) { + *tv = *timeout; + } else { + evutil_timerclear(tv); + } +} +static void +evhttp_set_timeout_(struct timeval *tv, int timeout, int def) +{ + if (timeout == -1) { + timeout = def; + } + + if (timeout == -1) { + evutil_timerclear(tv); + } else { + struct timeval timeout_tv; + timeout_tv.tv_sec = timeout; + timeout_tv.tv_usec = 0; + *tv = timeout_tv; + } +} + /* Called when evcon has experienced a (non-recoverable? -NM) error, as * given in error. If it's an outgoing connection, reset the connection, * retry any pending requests, and inform the user. If it's incoming, @@ -1615,13 +1647,8 @@ evhttp_connection_cb(struct bufferevent *bufev, short what, void *arg) evhttp_error_cb, evcon); - if (!evutil_timerisset(&evcon->timeout)) { - const struct timeval read_tv = { HTTP_READ_TIMEOUT, 0 }; - const struct timeval write_tv = { HTTP_WRITE_TIMEOUT, 0 }; - bufferevent_set_timeouts(evcon->bufev, &read_tv, &write_tv); - } else { - bufferevent_set_timeouts(evcon->bufev, &evcon->timeout, &evcon->timeout); - } + bufferevent_set_timeouts(evcon->bufev, + &evcon->timeout_read, &evcon->timeout_write); /* try to start requests that have queued up on this connection */ evhttp_request_dispatch(evcon); @@ -2378,7 +2405,11 @@ evhttp_connection_base_bufferevent_new(struct event_base *base, struct evdns_bas evcon->max_headers_size = EV_SIZE_MAX; evcon->max_body_size = EV_SIZE_MAX; - evutil_timerclear(&evcon->timeout); + evcon->timeout_connect.tv_sec = HTTP_CONNECT_TIMEOUT; + evcon->timeout_read.tv_sec = HTTP_READ_TIMEOUT; + evcon->timeout_write.tv_sec = HTTP_WRITE_TIMEOUT; + evcon->initial_retry_timeout.tv_sec = HTTP_INITIAL_RETRY_TIMEOUT; + evcon->retry_cnt = evcon->retry_max = 0; if ((evcon->address = mm_strdup(address)) == NULL) { @@ -2399,9 +2430,6 @@ evhttp_connection_base_bufferevent_new(struct event_base *base, struct evdns_bas evcon->state = EVCON_DISCONNECTED; TAILQ_INIT(&evcon->requests); - evcon->initial_retry_timeout.tv_sec = 2; - evcon->initial_retry_timeout.tv_usec = 0; - if (base != NULL) { evcon->base = base; if (bufferevent_get_base(bev) != base) @@ -2476,31 +2504,58 @@ evhttp_connection_set_base(struct evhttp_connection *evcon, void evhttp_connection_set_timeout(struct evhttp_connection *evcon, - int timeout_in_secs) + int timeout) { - if (timeout_in_secs == -1) - evhttp_connection_set_timeout_tv(evcon, NULL); - else { - struct timeval tv; - tv.tv_sec = timeout_in_secs; - tv.tv_usec = 0; - evhttp_connection_set_timeout_tv(evcon, &tv); + if (timeout != -1) { + evcon->flags |= EVHTTP_CON_TIMEOUT_ADJUSTED; + } else { + evcon->flags &= ~EVHTTP_CON_TIMEOUT_ADJUSTED; } + evhttp_set_timeout_(&evcon->timeout_read, timeout, HTTP_READ_TIMEOUT); + evhttp_set_timeout_(&evcon->timeout_write, timeout, HTTP_WRITE_TIMEOUT); + bufferevent_set_timeouts(evcon->bufev, + &evcon->timeout_read, &evcon->timeout_write); } - void evhttp_connection_set_timeout_tv(struct evhttp_connection *evcon, const struct timeval* tv) { if (tv) { - evcon->timeout = *tv; - bufferevent_set_timeouts(evcon->bufev, &evcon->timeout, &evcon->timeout); + evcon->flags |= EVHTTP_CON_TIMEOUT_ADJUSTED; } else { - const struct timeval read_tv = { HTTP_READ_TIMEOUT, 0 }; - const struct timeval write_tv = { HTTP_WRITE_TIMEOUT, 0 }; - evutil_timerclear(&evcon->timeout); - bufferevent_set_timeouts(evcon->bufev, &read_tv, &write_tv); + evcon->flags &= ~EVHTTP_CON_TIMEOUT_ADJUSTED; } + evhttp_set_timeout_tv_(&evcon->timeout_read, tv, HTTP_READ_TIMEOUT); + evhttp_set_timeout_tv_(&evcon->timeout_write, tv, HTTP_WRITE_TIMEOUT); + bufferevent_set_timeouts(evcon->bufev, + &evcon->timeout_read, &evcon->timeout_write); +} +void evhttp_connection_set_connect_timeout_tv(struct evhttp_connection *evcon, + const struct timeval *tv) +{ + evcon->flags |= EVHTTP_CON_TIMEOUT_ADJUSTED; + evhttp_set_timeout_tv_(&evcon->timeout_connect, tv, -1); + if (evcon->state == EVCON_CONNECTING) + bufferevent_set_timeouts(evcon->bufev, + &evcon->timeout_connect, &evcon->timeout_connect); +} +void evhttp_connection_set_read_timeout_tv(struct evhttp_connection *evcon, + const struct timeval *tv) +{ + evcon->flags |= EVHTTP_CON_TIMEOUT_ADJUSTED; + evhttp_set_timeout_tv_(&evcon->timeout_read, tv, -1); + if (evcon->state != EVCON_CONNECTING) + bufferevent_set_timeouts(evcon->bufev, + &evcon->timeout_read, &evcon->timeout_write); +} +void evhttp_connection_set_write_timeout_tv(struct evhttp_connection *evcon, + const struct timeval *tv) +{ + evcon->flags |= EVHTTP_CON_TIMEOUT_ADJUSTED; + evhttp_set_timeout_tv_(&evcon->timeout_write, tv, -1); + if (evcon->state != EVCON_CONNECTING) + bufferevent_set_timeouts(evcon->bufev, + &evcon->timeout_read, &evcon->timeout_write); } void @@ -2582,12 +2637,8 @@ evhttp_connection_connect_(struct evhttp_connection *evcon) NULL /* evhttp_write_cb */, evhttp_connection_cb, evcon); - if (!evutil_timerisset(&evcon->timeout)) { - const struct timeval conn_tv = { HTTP_CONNECT_TIMEOUT, 0 }; - bufferevent_set_timeouts(evcon->bufev, &conn_tv, &conn_tv); - } else { - bufferevent_set_timeouts(evcon->bufev, &evcon->timeout, &evcon->timeout); - } + bufferevent_set_timeouts(evcon->bufev, + &evcon->timeout_connect, &evcon->timeout_connect); /* make sure that we get a write callback */ if (bufferevent_enable(evcon->bufev, EV_WRITE)) return (-1); @@ -3668,7 +3719,9 @@ evhttp_new_object(void) return (NULL); } - evutil_timerclear(&http->timeout); + evutil_timerclear(&http->timeout_read); + evutil_timerclear(&http->timeout_write); + evhttp_set_max_headers_size(http, EV_SIZE_MAX); evhttp_set_max_body_size(http, EV_SIZE_MAX); evhttp_set_default_content_type(http, "text/html; charset=ISO-8859-1"); @@ -3838,26 +3891,26 @@ evhttp_remove_server_alias(struct evhttp *http, const char *alias) } void -evhttp_set_timeout(struct evhttp* http, int timeout_in_secs) +evhttp_set_timeout(struct evhttp* http, int timeout) { - if (timeout_in_secs == -1) { - evhttp_set_timeout_tv(http, NULL); - } else { - struct timeval tv; - tv.tv_sec = timeout_in_secs; - tv.tv_usec = 0; - evhttp_set_timeout_tv(http, &tv); - } + evhttp_set_timeout_(&http->timeout_read, timeout, -1); + evhttp_set_timeout_(&http->timeout_write, timeout, -1); } - void evhttp_set_timeout_tv(struct evhttp* http, const struct timeval* tv) { - if (tv) { - http->timeout = *tv; - } else { - evutil_timerclear(&http->timeout); - } + evhttp_set_timeout_tv_(&http->timeout_read, tv, -1); + evhttp_set_timeout_tv_(&http->timeout_write, tv, -1); +} +void +evhttp_set_read_timeout_tv(struct evhttp* http, const struct timeval* tv) +{ + evhttp_set_timeout_tv_(&http->timeout_read, tv, -1); +} +void +evhttp_set_write_timeout_tv(struct evhttp* http, const struct timeval* tv) +{ + evhttp_set_timeout_tv_(&http->timeout_write, tv, -1); } int evhttp_set_flags(struct evhttp *http, int flags) @@ -4330,8 +4383,10 @@ evhttp_get_request(struct evhttp *http, evutil_socket_t fd, } /* the timeout can be used by the server to close idle connections */ - if (evutil_timerisset(&http->timeout)) - evhttp_connection_set_timeout_tv(evcon, &http->timeout); + if (evutil_timerisset(&http->timeout_read)) + evhttp_connection_set_read_timeout_tv(evcon, &http->timeout_read); + if (evutil_timerisset(&http->timeout_write)) + evhttp_connection_set_write_timeout_tv(evcon, &http->timeout_write); /* * if we want to accept more than one request on a connection, diff --git a/include/event2/http.h b/include/event2/http.h index 534e604e..393c536f 100644 --- a/include/event2/http.h +++ b/include/event2/http.h @@ -378,20 +378,43 @@ int evhttp_remove_server_alias(struct evhttp *http, const char *alias); * Set the timeout for an HTTP request. * * @param http an evhttp object - * @param timeout_in_secs the timeout, in seconds + * @param timeout the timeout, in seconds + * @see evhttp_set_timeout_tv() */ EVENT2_EXPORT_SYMBOL -void evhttp_set_timeout(struct evhttp *http, int timeout_in_secs); +void evhttp_set_timeout(struct evhttp *http, int timeout); /** - * Set the timeout for an HTTP request. + * Set read and write timeout for an HTTP request. * * @param http an evhttp object * @param tv the timeout, or NULL + * + * For more precise control: + * @see evhttp_set_read_timeout_tv() + * @see evhttp_set_write_timeout_tv() */ EVENT2_EXPORT_SYMBOL void evhttp_set_timeout_tv(struct evhttp *http, const struct timeval* tv); +/** + * Set read timeout for an HTTP request. + * + * @param http an evhttp object + * @param tv the timeout, or NULL + */ +EVENT2_EXPORT_SYMBOL +void evhttp_set_read_timeout_tv(struct evhttp *http, const struct timeval* tv); + +/** + * Set write timeout for an HTTP request. + * + * @param http an evhttp object + * @param tv the timeout, or NULL + */ +EVENT2_EXPORT_SYMBOL +void evhttp_set_write_timeout_tv(struct evhttp *http, const struct timeval* tv); + /* Read all the clients body, and only after this respond with an error if the * clients body exceed max_body_size */ #define EVHTTP_SERVER_LINGERING_CLOSE 0x0001 @@ -527,6 +550,11 @@ enum evhttp_request_kind { EVHTTP_REQUEST, EVHTTP_RESPONSE }; * requests. The connection object tries to resolve address and establish the * connection when it is given an http request object. * + * Connection also has default timeouts for the following events: + * - connect HTTP_CONNECT_TIMEOUT, which is 45 seconds + * - read HTTP_READ_TIMEOUT which is 50 seconds + * - write HTTP_WRITE_TIMEOUT, which is 50 seconds + * * @param base the event_base to use for handling the connection * @param dnsbase the dns_base to use for resolving host names; if not * specified host name resolution will block. @@ -749,21 +777,69 @@ EVENT2_EXPORT_SYMBOL void evhttp_connection_set_local_port(struct evhttp_connection *evcon, ev_uint16_t port); -/** Sets the timeout in seconds for events related to this connection */ +/** + * Sets the timeout for this connection. + * + * @see evhttp_connection_set_timeout_tv() + */ EVENT2_EXPORT_SYMBOL void evhttp_connection_set_timeout(struct evhttp_connection *evcon, - int timeout_in_secs); + int timeout); -/** Sets the timeout for events related to this connection. Takes a struct - * timeval. */ +/** + * Sets the timeout for this connection for the following events: + * - read, if tv==NULL then it uses default timeout (HTTP_READ_TIMEOUT) + * - write, if tv==NULL then it uses default timeout (HTTP_WRITE_TIMEOUT) + * + * But it does not adjust timeout for the "connect" (for historical reasons). + * + * @param tv the timeout, or NULL + * + * For more precise control: + * @see evhttp_connection_set_connect_timeout_tv() + * @see evhttp_connection_set_read_timeout_tv() + * @see evhttp_connection_set_write_timeout_tv() + */ EVENT2_EXPORT_SYMBOL void evhttp_connection_set_timeout_tv(struct evhttp_connection *evcon, const struct timeval *tv); -/** Sets the delay before retrying requests on this connection. This is only - * used if evhttp_connection_set_retries is used to make the number of retries - * at least one. Each retry after the first is twice as long as the one before - * it. */ +/** + * Sets the connect timeout for this connection + * + * @param tv the timeout, or NULL + */ +EVENT2_EXPORT_SYMBOL +void evhttp_connection_set_connect_timeout_tv(struct evhttp_connection *evcon, + const struct timeval *tv); + +/** + * Sets the read timeout for this connection + * + * @param tv the timeout, or NULL + */ +EVENT2_EXPORT_SYMBOL +void evhttp_connection_set_read_timeout_tv(struct evhttp_connection *evcon, + const struct timeval *tv); + +/** + * Sets the write timeout for this connection + * + * @param tv the timeout, or NULL + */ +EVENT2_EXPORT_SYMBOL +void evhttp_connection_set_write_timeout_tv(struct evhttp_connection *evcon, + const struct timeval *tv); + +/** + * Sets the delay before retrying requests on this connection. + * + * This is only used if evhttp_connection_set_retries is used to make the + * number of retries at least one. Each retry after the first is twice as long + * as the one before it. + * + * Default delay is HTTP_INITIAL_RETRY_TIMEOUT, which is 2 seconds. + */ EVENT2_EXPORT_SYMBOL void evhttp_connection_set_initial_retry_tv(struct evhttp_connection *evcon, const struct timeval *tv); diff --git a/test/regress_http.c b/test/regress_http.c index f4e8bcd6..5e4e1a68 100644 --- a/test/regress_http.c +++ b/test/regress_http.c @@ -4829,6 +4829,94 @@ http_newreqcb_test(void *arg) } +static void +http_timeout_read_client_test(void *arg) +{ + struct basic_test_data *data = arg; + ev_uint16_t port = 0; + struct evhttp_connection *evcon = NULL; + struct evhttp_request *req = NULL; + struct timeval tv; + struct evhttp *http = http_setup(&port, data->base, 0); + + test_ok = 0; + exit_base = data->base; + + evcon = evhttp_connection_base_new(data->base, NULL, "127.0.0.1", port); + tt_assert(evcon); + + tv.tv_sec = 0; + tv.tv_usec = 100000; + evhttp_connection_set_connect_timeout_tv(evcon, &tv); + evhttp_connection_set_write_timeout_tv(evcon, &tv); + tv.tv_usec = 500000; + evhttp_connection_set_read_timeout_tv(evcon, &tv); + + req = evhttp_request_new(http_request_done, (void*)""); + tt_assert(req); + evhttp_add_header(evhttp_request_get_output_headers(req), "Host", "somehost"); + tt_int_op(evhttp_make_request(evcon, req, EVHTTP_REQ_GET, "/delay"), ==, 0); + event_base_dispatch(data->base); + tt_int_op(test_ok, ==, 1); + + end: + if (evcon) + evhttp_connection_free(evcon); + if (http) + evhttp_free(http); +} + +static void http_add_output_buffer(int fd, short events, void *arg) +{ + evbuffer_add(arg, POST_DATA, strlen(POST_DATA)); +} +static void +http_timeout_read_server_test(void *arg) +{ + struct basic_test_data *data = arg; + struct timeval tv; + struct bufferevent *bev; + struct evbuffer *out; + int fd = -1; + ev_uint16_t port = 0; + struct evhttp *http = http_setup(&port, data->base, 0); + + test_ok = 0; + + tv.tv_sec = 0; + tv.tv_usec = 100000; + evhttp_set_write_timeout_tv(http, &tv); + tv.tv_usec = 500000; + evhttp_set_read_timeout_tv(http, &tv); + + fd = http_connect("127.0.0.1", port); + bev = create_bev(data->base, fd, 0); + bufferevent_setcb(bev, http_readcb, http_writecb, http_errorcb, data->base); + out = bufferevent_get_output(bev); + + evbuffer_add_printf(out, + "POST /postit HTTP/1.1\r\n" + "Host: somehost\r\n" + "Content-Length: " EV_SIZE_FMT "\r\n" + "\r\n", strlen(POST_DATA)); + + tv.tv_usec = 200000; + event_base_once(data->base, -1, EV_TIMEOUT, http_add_output_buffer, out, &tv); + + event_base_dispatch(data->base); + tt_int_op(test_ok, ==, 3); + + end: + if (bev) + bufferevent_free(bev); + if (fd != -1) + evutil_closesocket(fd); + if (http) + evhttp_free(http); +} + + + #define HTTP_LEGACY(name) \ { #name, run_legacy_test_fn, TT_ISOLATED|TT_LEGACY, &legacy_setup, \ @@ -4959,6 +5047,9 @@ struct testcase_t http_testcases[] = { HTTP(newreqcb), + HTTP(timeout_read_client), + HTTP(timeout_read_server), + #ifdef EVENT__HAVE_OPENSSL HTTPS(basic), HTTPS(filter_basic),