packages/libevent
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

eliminate redundant bev fd manipulating and caching

Browse Source
This commit is contained in:
shixuantong 2023-09-18 16:20:46 +08:00
parent 347ef5e156
commit 19927002ad
4 changed files with 536 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

79
backport-bufferevent-introduce-bufferevent_replacefd-like-set.patch Normal file
View File

@ -0,0 +1,79 @@
From aea752b62dde0f02195b9c2143bdfcdfe65fb6fb Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <azat@libevent.org>
Date: Tue, 23 Mar 2021 09:00:24 +0300
Subject: [PATCH] bufferevent: introduce bufferevent_replacefd() (like setfd()
but also close fd)
Reference:https://github.com/libevent/libevent/commit/aea752b62dde0f02195b9c2143bdfcdfe65fb6fb
Conflict:NA
---
bufferevent.c | 28 ++++++++++++++++++++++++++++
include/event2/bufferevent.h | 12 ++++++++++++
2 files changed, 40 insertions(+)
diff --git a/bufferevent.c b/bufferevent.c
index 08c0486..68b35b1 100644
--- a/bufferevent.c
+++ b/bufferevent.c
@@ -876,6 +876,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 48cd153..e4e5c21 100644
--- a/include/event2/bufferevent.h
+++ b/include/event2/bufferevent.h
@@ -355,6 +355,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.
--
2.33.0

392
backport-http-eliminate-redundant-bev-fd-manipulating-and-cac.patch Normal file
View File

@ -0,0 +1,392 @@
From afa66ea4200a018bfc59abef8c2ffa11ef2b8363 Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <azat@libevent.org>
Date: Wed, 4 Sep 2019 00:56:20 +0300
Subject: [PATCH] http: eliminate redundant bev fd manipulating and caching
[WIP]
Reference:https://github.com/libevent/libevent/commit/afa66ea4200a018bfc59abef8c2ffa11ef2b8363
Conflict:NA
At the very beginning we reset the bufferevent fd (if bev has it), which
is not a good idea, since if user passes bufferevent with existing fd he
has some intention.
So we need to:
- use BEV_OPT_CLOSE_ON_FREE for default bufferevent_socket_new() (to
avoid manual shutdown/closee)
- drop getsockopt(SOL_SOCKET, SO_ERROR), since bufferevent already has
evutil_socket_finished_connecting_()
- drop supperior bufferevent_setfd(bev, -1) in
evhttp_connection_connect_()
Closes: #795
Refs: #875
---
http-internal.h | 3 +-
http.c | 146 +++++++++++++++++-------------------------
include/event2/http.h | 6 +-
3 files changed, 64 insertions(+), 91 deletions(-)
diff --git a/http-internal.h b/http-internal.h
index 9f9b5ab..bf54d61 100644
--- a/http-internal.h
+++ b/http-internal.h
@@ -53,7 +53,6 @@ struct evhttp_connection {
* server */
TAILQ_ENTRY(evhttp_connection) next;
- evutil_socket_t fd;
struct bufferevent *bufev;
struct event retry_ev; /* for retrying connects */
@@ -176,7 +175,7 @@ struct evhttp {
/* XXX most of these functions could be static. */
/* resets the connection; can be reused for more requests */
-void evhttp_connection_reset_(struct evhttp_connection *);
+void evhttp_connection_reset_(struct evhttp_connection *, int);
/* connects if necessary */
int evhttp_connection_connect_(struct evhttp_connection *);
diff --git a/http.c b/http.c
index 785def9..551b63b 100644
--- a/http.c
+++ b/http.c
@@ -777,7 +777,7 @@ evhttp_connection_fail_(struct evhttp_connection *evcon,
evhttp_request_free_(evcon, req);
/* reset the connection */
- evhttp_connection_reset_(evcon);
+ evhttp_connection_reset_(evcon, 1);
/* We are trying the next request that was queued on us */
if (TAILQ_FIRST(&evcon->requests) != NULL)
@@ -837,7 +837,7 @@ evhttp_connection_done(struct evhttp_connection *evcon)
/* check if we got asked to close the connection */
if (need_close)
- evhttp_connection_reset_(evcon);
+ evhttp_connection_reset_(evcon, 1);
if (TAILQ_FIRST(&evcon->requests) != NULL) {
/*
@@ -1171,7 +1171,7 @@ evhttp_read_cb(struct bufferevent *bufev, void *arg)
__func__, EV_SIZE_ARG(total_len)));
#endif
- evhttp_connection_reset_(evcon);
+ evhttp_connection_reset_(evcon, 1);
}
break;
case EVCON_DISCONNECTED:
@@ -1221,13 +1221,10 @@ void
evhttp_connection_free(struct evhttp_connection *evcon)
{
struct evhttp_request *req;
- int need_close = 0;
/* notify interested parties that this connection is going down */
- if (evcon->fd != -1) {
- if (evhttp_connected(evcon) && evcon->closecb != NULL)
- (*evcon->closecb)(evcon, evcon->closecb_arg);
- }
+ if (evhttp_connected(evcon) && evcon->closecb != NULL)
+ (*evcon->closecb)(evcon, evcon->closecb_arg);
/* remove all requests that might be queued on this
* connection. for server connections, this should be empty.
@@ -1252,20 +1249,9 @@ evhttp_connection_free(struct evhttp_connection *evcon)
&evcon->read_more_deferred_cb);
if (evcon->bufev != NULL) {
- need_close =
- !(bufferevent_get_options_(evcon->bufev) & BEV_OPT_CLOSE_ON_FREE);
- if (evcon->fd == -1)
- evcon->fd = bufferevent_getfd(evcon->bufev);
-
bufferevent_free(evcon->bufev);
}
- if (evcon->fd != -1) {
- shutdown(evcon->fd, EVUTIL_SHUT_WR);
- if (need_close)
- evutil_closesocket(evcon->fd);
- }
-
if (evcon->bind_address != NULL)
mm_free(evcon->bind_address);
@@ -1324,16 +1310,19 @@ evhttp_request_dispatch(struct evhttp_connection* evcon)
evhttp_write_buffer(evcon, evhttp_write_connectioncb, NULL);
}
-/* Reset our connection state: disables reading/writing, closes our fd (if
-* any), clears out buffers, and puts us in state DISCONNECTED. */
-void
-evhttp_connection_reset_(struct evhttp_connection *evcon)
+/** Hard-reset our connection state
+ *
+ * This will:
+ * - reset fd
+ * - clears out buffers
+ * - call closecb
+ */
+static void
+evhttp_connection_reset_hard_(struct evhttp_connection *evcon)
{
struct evbuffer *tmp;
int err;
- bufferevent_setcb(evcon->bufev, NULL, NULL, NULL, NULL);
-
/* 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
connecting. But for Libevent 2.0, this seems like a minimal change
@@ -1347,18 +1336,11 @@ evhttp_connection_reset_(struct evhttp_connection *evcon)
*/
bufferevent_disable_hard_(evcon->bufev, EV_READ|EV_WRITE);
- if (evcon->fd == -1)
- evcon->fd = bufferevent_getfd(evcon->bufev);
-
- if (evcon->fd != -1) {
- /* inform interested parties about connection close */
- if (evhttp_connected(evcon) && evcon->closecb != NULL)
- (*evcon->closecb)(evcon, evcon->closecb_arg);
+ /* inform interested parties about connection close */
+ if (evhttp_connected(evcon) && evcon->closecb != NULL)
+ (*evcon->closecb)(evcon, evcon->closecb_arg);
- shutdown(evcon->fd, EVUTIL_SHUT_WR);
- evutil_closesocket(evcon->fd);
- evcon->fd = -1;
- }
+ /** FIXME: manipulating with fd is unwanted */
err = bufferevent_setfd(evcon->bufev, -1);
EVUTIL_ASSERT(!err && "setfd");
@@ -1369,9 +1351,26 @@ evhttp_connection_reset_(struct evhttp_connection *evcon)
tmp = bufferevent_get_input(evcon->bufev);
err = evbuffer_drain(tmp, -1);
EVUTIL_ASSERT(!err && "drain input");
+}
- evcon->flags &= ~EVHTTP_CON_READING_ERROR;
+/** Reset our connection state
+ *
+ * This will:
+ * - disables reading/writing
+ * - puts us in DISCONNECTED state
+ *
+ * @param hard - hard reset will (@see evhttp_connection_reset_hard_())
+ */
+void
+evhttp_connection_reset_(struct evhttp_connection *evcon, int hard)
+{
+ bufferevent_setcb(evcon->bufev, NULL, NULL, NULL, NULL);
+ if (hard) {
+ evhttp_connection_reset_hard_(evcon);
+ }
+
+ evcon->flags &= ~EVHTTP_CON_READING_ERROR;
evcon->state = EVCON_DISCONNECTED;
}
@@ -1403,7 +1402,7 @@ evhttp_connection_cb_cleanup(struct evhttp_connection *evcon)
{
struct evcon_requestq requests;
- evhttp_connection_reset_(evcon);
+ evhttp_connection_reset_(evcon, 1);
if (evcon->retry_max < 0 || evcon->retry_cnt < evcon->retry_max) {
struct timeval tv_retry = evcon->initial_retry_timeout;
int i;
@@ -1481,16 +1480,13 @@ evhttp_error_cb(struct bufferevent *bufev, short what, void *arg)
struct evhttp_connection *evcon = arg;
struct evhttp_request *req = TAILQ_FIRST(&evcon->requests);
- if (evcon->fd == -1)
- evcon->fd = bufferevent_getfd(bufev);
-
switch (evcon->state) {
case EVCON_CONNECTING:
if (what & BEV_EVENT_TIMEOUT) {
event_debug(("%s: connection timeout for \"%s:%d\" on "
EV_SOCK_FMT,
__func__, evcon->address, evcon->port,
- EV_SOCK_ARG(evcon->fd)));
+ EV_SOCK_ARG(bufferevent_getfd(bufev))));
evhttp_connection_cb_cleanup(evcon);
return;
}
@@ -1526,7 +1522,7 @@ evhttp_error_cb(struct bufferevent *bufev, short what, void *arg)
* disconnected.
*/
EVUTIL_ASSERT(evcon->state == EVCON_IDLE);
- evhttp_connection_reset_(evcon);
+ evhttp_connection_reset_(evcon, 1);
/*
* If we have no more requests that need completion
@@ -1572,11 +1568,6 @@ static void
evhttp_connection_cb(struct bufferevent *bufev, short what, void *arg)
{
struct evhttp_connection *evcon = arg;
- int error;
- ev_socklen_t errsz = sizeof(error);
-
- if (evcon->fd == -1)
- evcon->fd = bufferevent_getfd(bufev);
if (!(what & BEV_EVENT_CONNECTED)) {
/* some operating systems return ECONNREFUSED immediately
@@ -1591,34 +1582,10 @@ evhttp_connection_cb(struct bufferevent *bufev, short what, void *arg)
return;
}
- if (evcon->fd == -1) {
- event_debug(("%s: bufferevent_getfd returned -1",
- __func__));
- goto cleanup;
- }
-
- /* Check if the connection completed */
- if (getsockopt(evcon->fd, SOL_SOCKET, SO_ERROR, (void*)&error,
- &errsz) == -1) {
- event_debug(("%s: getsockopt for \"%s:%d\" on "EV_SOCK_FMT,
- __func__, evcon->address, evcon->port,
- EV_SOCK_ARG(evcon->fd)));
- goto cleanup;
- }
-
- if (error) {
- event_debug(("%s: connect failed for \"%s:%d\" on "
- EV_SOCK_FMT": %s",
- __func__, evcon->address, evcon->port,
- EV_SOCK_ARG(evcon->fd),
- evutil_socket_error_to_string(error)));
- goto cleanup;
- }
-
/* We are connected to the server now */
event_debug(("%s: connected to \"%s:%d\" on "EV_SOCK_FMT"\n",
__func__, evcon->address, evcon->port,
- EV_SOCK_ARG(evcon->fd)));
+ EV_SOCK_ARG(bufferevent_getfd(bufev))));
/* Reset the retry count as we were successful in connecting */
evcon->retry_cnt = 0;
@@ -2280,7 +2247,7 @@ evhttp_read_firstline(struct evhttp_connection *evcon,
if (res == DATA_CORRUPTED || res == DATA_TOO_LONG) {
/* Error while reading, terminate */
event_debug(("%s: bad header lines on "EV_SOCK_FMT"\n",
- __func__, EV_SOCK_ARG(evcon->fd)));
+ __func__, EV_SOCK_ARG(bufferevent_getfd(evcon->bufev))));
evhttp_connection_fail_(evcon, EVREQ_HTTP_INVALID_HEADER);
return;
} else if (res == MORE_DATA_EXPECTED) {
@@ -2297,7 +2264,7 @@ evhttp_read_header(struct evhttp_connection *evcon,
struct evhttp_request *req)
{
enum message_read_status res;
- evutil_socket_t fd = evcon->fd;
+ evutil_socket_t fd = bufferevent_getfd(evcon->bufev);
res = evhttp_parse_headers_(req, bufferevent_get_input(evcon->bufev));
if (res == DATA_CORRUPTED || res == DATA_TOO_LONG) {
@@ -2388,7 +2355,6 @@ evhttp_connection_base_bufferevent_new(struct event_base *base, struct evdns_bas
goto error;
}
- evcon->fd = -1;
evcon->port = port;
evcon->max_headers_size = EV_SIZE_MAX;
@@ -2403,7 +2369,7 @@ evhttp_connection_base_bufferevent_new(struct event_base *base, struct evdns_bas
}
if (bev == NULL) {
- if (!(bev = bufferevent_socket_new(base, -1, 0))) {
+ if (!(bev = bufferevent_socket_new(base, -1, BEV_OPT_CLOSE_ON_FREE))) {
event_warn("%s: bufferevent_socket_new failed", __func__);
goto error;
}
@@ -2571,24 +2537,30 @@ evhttp_connection_connect_(struct evhttp_connection *evcon)
if (evcon->state == EVCON_CONNECTING)
return (0);
- evhttp_connection_reset_(evcon);
+ /* Do not do hard reset, since this will reset the fd, but someone may
+ * change some options for it (i.e. setsockopt(), #875)
+ *
+ * However don't think that this options will be preserved for all
+ * connection lifetime, they will be reseted in the following cases:
+ * - evhttp_connection_set_local_address()
+ * - evhttp_connection_set_local_port()
+ * - evhttp_connection_set_retries()
+ * */
+ evhttp_connection_reset_(evcon, 0);
EVUTIL_ASSERT(!(evcon->flags & EVHTTP_CON_INCOMING));
evcon->flags |= EVHTTP_CON_OUTGOING;
if (evcon->bind_address || evcon->bind_port) {
- evcon->fd = bind_socket(
- evcon->bind_address, evcon->bind_port, 0 /*reuse*/);
- if (evcon->fd == -1) {
+ int fd = bind_socket(evcon->bind_address, evcon->bind_port,
+ 0 /*reuse*/);
+ if (fd == -1) {
event_debug(("%s: failed to bind to \"%s\"",
__func__, evcon->bind_address));
return (-1);
}
- if (bufferevent_setfd(evcon->bufev, evcon->fd))
- return (-1);
- } else {
- if (bufferevent_setfd(evcon->bufev, -1))
+ if (bufferevent_setfd(evcon->bufev, fd))
return (-1);
}
@@ -2625,7 +2597,7 @@ evhttp_connection_connect_(struct evhttp_connection *evcon)
if (ret < 0) {
evcon->state = old_state;
- event_sock_warn(evcon->fd, "%s: connection to \"%s\" failed",
+ event_sock_warn(bufferevent_getfd(evcon->bufev), "%s: connection to \"%s\" failed",
__func__, evcon->address);
/* some operating systems return ECONNREFUSED immediately
* when connecting to a local address. the cleanup is going
@@ -4273,8 +4245,6 @@ evhttp_get_request_connection(
evcon->flags |= EVHTTP_CON_INCOMING;
evcon->state = EVCON_READING_FIRSTLINE;
- evcon->fd = fd;
-
if (bufferevent_setfd(evcon->bufev, fd))
goto err;
if (bufferevent_enable(evcon->bufev, EV_READ))
diff --git a/include/event2/http.h b/include/event2/http.h
index ed9acf4..c1521ac 100644
--- a/include/event2/http.h
+++ b/include/event2/http.h
@@ -739,7 +739,11 @@ void evhttp_connection_free(struct evhttp_connection *evcon);
EVENT2_EXPORT_SYMBOL
void evhttp_connection_free_on_completion(struct evhttp_connection *evcon);
-/** sets the ip address from which http connections are made */
+/** Sets the IP address from which http connections are made
+ *
+ * Note this resets internal bufferevent fd, so any options that had been
+ * installed will be flushed.
+ */
EVENT2_EXPORT_SYMBOL
void evhttp_connection_set_local_address(struct evhttp_connection *evcon,
const char *address);
--
2.33.0

58
backport-http-fix-fd-leak-on-fd-reset-by-using-bufferevent_re.patch Normal file
View File

@ -0,0 +1,58 @@
From 2385638edf9cb833ebc2759cdb6d6d45dc51a0da Mon Sep 17 00:00:00 2001
From: Azat Khuzhin <azat@libevent.org>
Date: Tue, 23 Mar 2021 09:02:39 +0300
Subject: [PATCH] http: fix fd leak on fd reset (by using
bufferevent_replacefd())
Reference:https://github.com/libevent/libevent/commit/2385638edf9cb833ebc2759cdb6d6d45dc51a0da
Conflict:NA
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 551b63b..420049a 100644
--- a/http.c
+++ b/http.c
@@ -1324,7 +1324,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.
@@ -1341,7 +1341,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 */
@@ -2560,7 +2560,7 @@ evhttp_connection_connect_(struct evhttp_connection *evcon)
return (-1);
}
- if (bufferevent_setfd(evcon->bufev, fd))
+ if (bufferevent_replacefd(evcon->bufev, fd))
return (-1);
}
@@ -4245,7 +4245,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;
--
2.33.0

8
libevent.spec
View File

@ -1,6 +1,6 @@
Name: libevent
Version: 2.1.12
Release: 8
Release: 9
Summary: An event notification library
License: BSD
@ -19,6 +19,9 @@ Patch2: add-testcases-for-event.c-apis.patch
# https://github.com/transmission/transmission/issues/1437
Patch3: 0001-Revert-Fix-checking-return-value-of-the-evdns_base_r.patch
Patch6000: backport-ssl-do-not-trigger-EOF-if-some-data-had-been-successf.patch
Patch6001: backport-http-eliminate-redundant-bev-fd-manipulating-and-cac.patch
Patch6002: backport-http-fix-fd-leak-on-fd-reset-by-using-bufferevent_re.patch
Patch6003: backport-bufferevent-introduce-bufferevent_replacefd-like-set.patch
%description
Libevent additionally provides a sophisticated framework for buffered network IO, with support for sockets,
@ -79,6 +82,9 @@ rm -f %{buildroot}%{_libdir}/*.la
%changelog
* Mon Sep 18 2023 shixuantong <shixuantong1@huawei.com> - 2.1.12-9
- eliminate redundant bev fd manipulating and caching
* Sat Jul 29 2023 shixuantong <shixuantong1@huawei.com> - 2.1.12-8
- ssl: do not trigger EOF if some data had been successfully read