Skip to content

Commit

Permalink
api: fix deadlock on shutdown when clients are connected
Browse files Browse the repository at this point in the history
The docstring of event_base_foreach_event() states explicitly that
modifying events in the callback function is unsafe and will cause
crashes.

event_free_finalize tries to acquire a lock that is already held when
the callback is called. Leading to a deadlock:

 (gdb) bt
 ...
 DPDK#3  ___pthread_mutex_lock (mutex=0x504000001550) at pthread_mutex_lock.c:93
 DPDK#4  0x00007f3f211b8485 in event_finalize_impl_ (flags=65536, ev=0x50c000005200, cb=0x4031ba <finalize_fd>)
 DPDK#5  0x00000000004042ed in close_connections (ev=0x50c000005200) at main/api.c:174
 ...
 DPDK#10 0x00007f3f211b263a in event_base_foreach_event (base=0x517000006d00, fn=0x40429f <close_connections>, arg=0x0)
 DPDK#11 0x0000000000404a3a in api_socket_stop () at main/api.c:253
 DPDK#12 0x00000000004072aa in main (argc=4, argv=0x7ffcc575d898) at main/main.c:210

Only use event_base_foreach_event() for iterating over the events that
we actually want to free (namely, ones that have read_cb and write_cb as
callbacks).

Only *after* returning from event_base_foreach_event(), call
event_free_finalize on all these events.

Fixes: 8653320 ("main: close active connections on shutdown")
Signed-off-by: Robin Jarry <[email protected]>
  • Loading branch information
rjarry committed Dec 6, 2024
1 parent 5c91e81 commit 127b571
Showing 1 changed file with 16 additions and 8 deletions.
24 changes: 16 additions & 8 deletions main/api.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <gr_api.h>
#include <gr_log.h>
#include <gr_macro.h>
#include <gr_vec.h>

#include <event2/event.h>

Expand Down Expand Up @@ -168,13 +169,6 @@ static void read_cb(evutil_socket_t sock, short what, void * /*priv*/) {
event_free_finalize(0, ev, finalize_fd);
}

static int close_connections(const struct event_base *, const struct event *ev, void * /*priv*/) {
event_callback_fn cb = event_get_callback(ev);
if (cb == read_cb || cb == write_cb)
event_free_finalize(0, (struct event *)ev, finalize_fd);
return 0;
}

static void listen_cb(evutil_socket_t sock, short what, void * /*priv*/) {
struct event *ev;
int fd;
Expand Down Expand Up @@ -246,9 +240,23 @@ int api_socket_start(struct event_base *base) {
return 0;
}

static int collect_clients(const struct event_base *, const struct event *ev, void *priv) {
struct event ***events = priv;
event_callback_fn cb = event_get_callback(ev);
if (cb == read_cb || cb == write_cb)
gr_vec_add(*events, (struct event *)ev);
return 0;
}

void api_socket_stop(struct event_base *) {
struct event **events = NULL;
struct event *ev;

if (ev_listen != NULL)
event_free_finalize(0, ev_listen, finalize_fd);

event_base_foreach_event(ev_base, close_connections, NULL);
event_base_foreach_event(ev_base, collect_clients, &events);
gr_vec_foreach (ev, events)
event_free_finalize(0, ev, finalize_fd);
gr_vec_free(events);
}

0 comments on commit 127b571

Please sign in to comment.