From a96b73b9fd3a5197c58b3e5b3a9697f8383bcc6c Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 5 Nov 2015 17:35:17 +0300 Subject: [PATCH 1/5] be: add_event: use evutil_timerisset() --- bufferevent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bufferevent.c b/bufferevent.c index 7d3d6935..a1fa6227 100644 --- a/bufferevent.c +++ b/bufferevent.c @@ -972,7 +972,7 @@ bufferevent_generic_adj_timeouts_(struct bufferevent *bev) int bufferevent_add_event_(struct event *ev, const struct timeval *tv) { - if (tv->tv_sec == 0 && tv->tv_usec == 0) + if (!evutil_timerisset(tv)) return event_add(ev, NULL); else return event_add(ev, tv); From f4b6284b8393dbabf389ddce734a30f4cdeffa17 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 5 Nov 2015 17:40:25 +0300 Subject: [PATCH 2/5] be_openssl: don't add events during bev creation (like be_sock) Using the following examples you can get changes between be_openssl and be_sock: $ function diff_addr() { eval diff -u $(printf "<(strip_addr %s) " "$@") } $ function strip_addr() { sed 's/0x[a-zA-Z0-9]*/0xFFFF/g' "$@" } $ EVENT_DEBUG_LOGGING_ALL= regress --verbose --no-fork +http/https_connection_retry 2> /tmp/https-retry.log >&2 $ EVENT_DEBUG_LOGGING_ALL= regress --verbose --no-fork +http/connection_retry 2> /tmp/http-retry.log >&2 $ diff_addr /tmp/http-retry.log /tmp/https-retry.log --- bufferevent_openssl.c | 9 --------- 1 file changed, 9 deletions(-) diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c index 4afdde27..7d469bfa 100644 --- a/bufferevent_openssl.c +++ b/bufferevent_openssl.c @@ -1394,15 +1394,6 @@ bufferevent_openssl_new_impl(struct event_base *base, if (state == BUFFEREVENT_SSL_OPEN) bufferevent_suspend_read_(underlying, BEV_SUSPEND_FILT_READ); - } else { - struct bufferevent *bev = &bev_ssl->bev.bev; - bev->enabled = EV_READ|EV_WRITE; - if (state != BUFFEREVENT_SSL_OPEN) - if (event_add(&bev->ev_read, NULL) < 0) - goto err; - if (event_initialized(&bev->ev_write)) - if (event_add(&bev->ev_write, NULL) < 0) - goto err; } return &bev_ssl->bev.bev; From 3c1f58f58b5ecfb59476b3e99629cfce20d16f91 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 5 Nov 2015 17:45:09 +0300 Subject: [PATCH 3/5] be: introduce bufferevent_generic_adj_existing_timeouts_() And use it in openssl/sock layers to avoid copy-pasting it's variants. --- bufferevent-internal.h | 1 + bufferevent.c | 23 +++++++++++++++++++++++ bufferevent_openssl.c | 18 +----------------- bufferevent_sock.c | 26 +------------------------- 4 files changed, 26 insertions(+), 42 deletions(-) diff --git a/bufferevent-internal.h b/bufferevent-internal.h index c84f09c0..d9d9e666 100644 --- a/bufferevent-internal.h +++ b/bufferevent-internal.h @@ -412,6 +412,7 @@ void bufferevent_init_generic_timeout_cbs_(struct bufferevent *bev); * we delete it.) Call this from anything that changes the timeout values, * that enabled EV_READ or EV_WRITE, or that disables EV_READ or EV_WRITE. */ int bufferevent_generic_adj_timeouts_(struct bufferevent *bev); +int bufferevent_generic_adj_existing_timeouts_(struct bufferevent *bev); enum bufferevent_options bufferevent_get_options_(struct bufferevent *bev); diff --git a/bufferevent.c b/bufferevent.c index a1fa6227..59ae24f1 100644 --- a/bufferevent.c +++ b/bufferevent.c @@ -969,6 +969,29 @@ bufferevent_generic_adj_timeouts_(struct bufferevent *bev) return 0; } +int +bufferevent_generic_adj_existing_timeouts_(struct bufferevent *bev) +{ + int r = 0; + if (event_pending(&bev->ev_read, EV_READ, NULL)) { + if (evutil_timerisset(&bev->timeout_read)) { + if (bufferevent_add_event_(&bev->ev_read, &bev->timeout_read) < 0) + r = -1; + } else { + event_remove_timer(&bev->ev_read); + } + } + if (event_pending(&bev->ev_write, EV_WRITE, NULL)) { + if (evutil_timerisset(&bev->timeout_write)) { + if (bufferevent_add_event_(&bev->ev_write, &bev->timeout_write) < 0) + r = -1; + } else { + event_remove_timer(&bev->ev_write); + } + } + return r; +} + int bufferevent_add_event_(struct event *ev, const struct timeval *tv) { diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c index 7d469bfa..54b72197 100644 --- a/bufferevent_openssl.c +++ b/bufferevent_openssl.c @@ -1254,23 +1254,7 @@ be_openssl_adj_timeouts(struct bufferevent *bev) if (bev_ssl->underlying) { return bufferevent_generic_adj_timeouts_(bev); } else { - int r1=0, r2=0; - if (event_pending(&bev->ev_read, EV_READ, NULL)) { - if (evutil_timerisset(&bev->timeout_read)) { - r1 = bufferevent_add_event_(&bev->ev_read, &bev->timeout_read); - } else { - event_remove_timer(&bev->ev_read); - } - } - if (event_pending(&bev->ev_write, EV_WRITE, NULL)) { - if (evutil_timerisset(&bev->timeout_write)) { - r2 = bufferevent_add_event_(&bev->ev_write, &bev->timeout_write); - } else { - event_remove_timer(&bev->ev_write); - } - } - - return (r1<0 || r2<0) ? -1 : 0; + return bufferevent_generic_adj_existing_timeouts_(bev); } } diff --git a/bufferevent_sock.c b/bufferevent_sock.c index ffc6c772..431d6e4b 100644 --- a/bufferevent_sock.c +++ b/bufferevent_sock.c @@ -79,7 +79,6 @@ static int be_socket_enable(struct bufferevent *, short); static int be_socket_disable(struct bufferevent *, short); static void be_socket_destruct(struct bufferevent *); -static int be_socket_adj_timeouts(struct bufferevent *); static int be_socket_flush(struct bufferevent *, short, enum bufferevent_flush_mode); static int be_socket_ctrl(struct bufferevent *, enum bufferevent_ctrl_op, union bufferevent_ctrl_data *); @@ -92,7 +91,7 @@ const struct bufferevent_ops bufferevent_ops_socket = { be_socket_disable, NULL, /* unlink */ be_socket_destruct, - be_socket_adj_timeouts, + bufferevent_generic_adj_existing_timeouts_, be_socket_flush, be_socket_ctrl, }; @@ -619,29 +618,6 @@ be_socket_destruct(struct bufferevent *bufev) EVUTIL_CLOSESOCKET(fd); } -static int -be_socket_adj_timeouts(struct bufferevent *bufev) -{ - int r = 0; - if (event_pending(&bufev->ev_read, EV_READ, NULL)) { - if (evutil_timerisset(&bufev->timeout_read)) { - if (be_socket_add(&bufev->ev_read, &bufev->timeout_read) < 0) - r = -1; - } else { - event_remove_timer(&bufev->ev_read); - } - } - if (event_pending(&bufev->ev_write, EV_WRITE, NULL)) { - if (evutil_timerisset(&bufev->timeout_write)) { - if (be_socket_add(&bufev->ev_write, &bufev->timeout_write) < 0) - r = -1; - } else { - event_remove_timer(&bufev->ev_write); - } - } - return r; -} - static int be_socket_flush(struct bufferevent *bev, short iotype, enum bufferevent_flush_mode mode) From fad5fe2cc1c7826decc37df4a37cac45f8fd1b78 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 5 Nov 2015 17:51:46 +0300 Subject: [PATCH 4/5] be_sock: drop be_sock_add() macro (useless and debug unfriendly) --- bufferevent_sock.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/bufferevent_sock.c b/bufferevent_sock.c index 431d6e4b..5c014426 100644 --- a/bufferevent_sock.c +++ b/bufferevent_sock.c @@ -96,9 +96,6 @@ const struct bufferevent_ops bufferevent_ops_socket = { be_socket_ctrl, }; -#define be_socket_add(ev, t) \ - bufferevent_add_event_((ev), (t)) - const struct sockaddr* bufferevent_socket_get_conn_address_(struct bufferevent *bev) { @@ -139,7 +136,7 @@ bufferevent_socket_outbuf_cb(struct evbuffer *buf, !bufev_p->write_suspended) { /* Somebody added data to the buffer, and we would like to * write, and we were not writing. So, start writing. */ - if (be_socket_add(&bufev->ev_write, &bufev->timeout_write) == -1) { + if (bufferevent_add_event_(&bufev->ev_write, &bufev->timeout_write) == -1) { /* Should we log this? */ } } @@ -577,11 +574,11 @@ static int be_socket_enable(struct bufferevent *bufev, short event) { if (event & EV_READ) { - if (be_socket_add(&bufev->ev_read,&bufev->timeout_read) == -1) + if (bufferevent_add_event_(&bufev->ev_read, &bufev->timeout_read) == -1) return -1; } if (event & EV_WRITE) { - if (be_socket_add(&bufev->ev_write,&bufev->timeout_write) == -1) + if (bufferevent_add_event_(&bufev->ev_write, &bufev->timeout_write) == -1) return -1; } return 0; From 0c66d3210c348c9f0b4db1ae8ebce29ab3606d70 Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Thu, 5 Nov 2015 17:56:07 +0300 Subject: [PATCH 5/5] be_openssl: use bufferevent_enable() instead of bufferevent_add_event_() By using bufferevent_enable() there will be no event for READ *or* WRITE if they are not enabled before, and this patch reduces difference for be_sock_enable/be_openssl_enable (handshake) --- bufferevent_openssl.c | 12 +++--------- bufferevent_sock.c | 10 ++++------ 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/bufferevent_openssl.c b/bufferevent_openssl.c index 54b72197..7e4d1744 100644 --- a/bufferevent_openssl.c +++ b/bufferevent_openssl.c @@ -1090,7 +1090,6 @@ set_handshake_callbacks(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd) return do_handshake(bev_ssl); } else { struct bufferevent *bev = &bev_ssl->bev.bev; - int r1=0, r2=0; if (event_initialized(&bev->ev_read)) { event_del(&bev->ev_read); @@ -1103,11 +1102,9 @@ set_handshake_callbacks(struct bufferevent_openssl *bev_ssl, evutil_socket_t fd) event_assign(&bev->ev_write, bev->ev_base, fd, EV_WRITE|EV_PERSIST|EV_FINALIZE, be_openssl_handshakeeventcb, bev_ssl); - if (fd >= 0) { - r1 = bufferevent_add_event_(&bev->ev_read, &bev->timeout_read); - r2 = bufferevent_add_event_(&bev->ev_write, &bev->timeout_write); - } - return (r1<0 || r2<0) ? -1 : 0; + if (fd >= 0) + bufferevent_enable(bev, bev->enabled); + return 0; } } @@ -1159,9 +1156,6 @@ be_openssl_enable(struct bufferevent *bev, short events) struct bufferevent_openssl *bev_ssl = upcast(bev); int r1 = 0, r2 = 0; - if (bev_ssl->state != BUFFEREVENT_SSL_OPEN) - return 0; - if (events & EV_READ) r1 = start_reading(bev_ssl); if (events & EV_WRITE) diff --git a/bufferevent_sock.c b/bufferevent_sock.c index 5c014426..eb731484 100644 --- a/bufferevent_sock.c +++ b/bufferevent_sock.c @@ -573,14 +573,12 @@ bufferevent_new(evutil_socket_t fd, static int be_socket_enable(struct bufferevent *bufev, short event) { - if (event & EV_READ) { - if (bufferevent_add_event_(&bufev->ev_read, &bufev->timeout_read) == -1) + if (event & EV_READ && + bufferevent_add_event_(&bufev->ev_read, &bufev->timeout_read) == -1) return -1; - } - if (event & EV_WRITE) { - if (bufferevent_add_event_(&bufev->ev_write, &bufev->timeout_write) == -1) + if (event & EV_WRITE && + bufferevent_add_event_(&bufev->ev_write, &bufev->timeout_write) == -1) return -1; - } return 0; }