From aea752b62dde0f02195b9c2143bdfcdfe65fb6fb Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 23 Mar 2021 09:00:24 +0300 Subject: [PATCH 1/2] bufferevent: introduce bufferevent_replacefd() (like setfd() but also close fd) --- bufferevent.c | 28 ++++++++++++++++++++++++++++ include/event2/bufferevent.h | 12 ++++++++++++ 2 files changed, 40 insertions(+) diff --git a/bufferevent.c b/bufferevent.c index 27f2a9ba..53d3a995 100644 --- a/bufferevent.c +++ b/bufferevent.c @@ -881,6 +881,34 @@ bufferevent_setfd(struct bufferevent *bev, evutil_socket_t fd) return res; } +int +bufferevent_replacefd(struct bufferevent *bev, evutil_socket_t fd) +{ + union bufferevent_ctrl_data d; + int err = -1; + evutil_socket_t old_fd = EVUTIL_INVALID_SOCKET; + + BEV_LOCK(bev); + if (bev->be_ops->ctrl) { + err = bev->be_ops->ctrl(bev, BEV_CTRL_GET_FD, &d); + if (!err) { + old_fd = d.fd; + if (old_fd != EVUTIL_INVALID_SOCKET) { + err = evutil_closesocket(old_fd); + } + } + if (!err) { + d.fd = fd; + err = bev->be_ops->ctrl(bev, BEV_CTRL_SET_FD, &d); + } + } + if (err) + event_debug(("%s: cannot replace fd for %p from "EV_SOCK_FMT" to "EV_SOCK_FMT, __func__, bev, old_fd, fd)); + BEV_UNLOCK(bev); + + return err; +} + evutil_socket_t bufferevent_getfd(struct bufferevent *bev) { diff --git a/include/event2/bufferevent.h b/include/event2/bufferevent.h index 58baf831..a50944f7 100644 --- a/include/event2/bufferevent.h +++ b/include/event2/bufferevent.h @@ -377,6 +377,18 @@ void bufferevent_getcb(struct bufferevent *bufev, EVENT2_EXPORT_SYMBOL int bufferevent_setfd(struct bufferevent *bufev, evutil_socket_t fd); +/** + Replaces the file descriptor on which the bufferevent operates. + Not supported for all bufferevent types. + + Unlike bufferevent_setfd() it will close previous file descriptor (if any). + + @param bufev the bufferevent object for which to change the file descriptor + @param fd the file descriptor to operate on +*/ +EVENT2_EXPORT_SYMBOL +int bufferevent_replacefd(struct bufferevent *bufev, evutil_socket_t fd); + /** Returns the file descriptor associated with a bufferevent, or -1 if no file descriptor is associated with the bufferevent. From 2385638edf9cb833ebc2759cdb6d6d45dc51a0da Mon Sep 17 00:00:00 2001 From: Azat Khuzhin Date: Tue, 23 Mar 2021 09:02:39 +0300 Subject: [PATCH 2/2] http: fix fd leak on fd reset (by using bufferevent_replacefd()) Fixes: afa66ea4 ("http: eliminate redundant bev fd manipulating and caching [WIP]") --- http.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/http.c b/http.c index 9dcb1696..28b59b88 100644 --- a/http.c +++ b/http.c @@ -1420,7 +1420,7 @@ evhttp_connection_reset_hard_(struct evhttp_connection *evcon) int err; /* XXXX This is not actually an optimal fix. Instead we ought to have - an API for "stop connecting", or use bufferevent_setfd to turn off + an API for "stop connecting", or use bufferevent_replacefd to turn off connecting. But for Libevent 2.0, this seems like a minimal change least likely to disrupt the rest of the bufferevent and http code. @@ -1437,7 +1437,7 @@ evhttp_connection_reset_hard_(struct evhttp_connection *evcon) (*evcon->closecb)(evcon, evcon->closecb_arg); /** FIXME: manipulating with fd is unwanted */ - err = bufferevent_setfd(evcon->bufev, -1); + err = bufferevent_replacefd(evcon->bufev, -1); EVUTIL_ASSERT(!err && "setfd"); /* we need to clean up any buffered data */ @@ -2766,7 +2766,7 @@ evhttp_connection_connect_(struct evhttp_connection *evcon) return (-1); } - if (bufferevent_setfd(evcon->bufev, fd)) + if (bufferevent_replacefd(evcon->bufev, fd)) return (-1); } @@ -4495,7 +4495,7 @@ evhttp_get_request_connection( evcon->flags |= EVHTTP_CON_INCOMING; evcon->state = EVCON_READING_FIRSTLINE; - if (bufferevent_setfd(evcon->bufev, fd)) + if (bufferevent_replacefd(evcon->bufev, fd)) goto err; if (bufferevent_enable(evcon->bufev, EV_READ)) goto err;