From 4335d9394625c3f3cf424874eeaaaf0f52b33b10 Mon Sep 17 00:00:00 2001 From: Alexandre Antonio Juca Date: Mon, 27 Apr 2026 22:18:44 +0100 Subject: [PATCH] fix: centralize client drop cleanup --- src/io/event_dispatcher_epoll.c | 16 +-------- src/io/event_dispatcher_io_uring.c | 16 +-------- src/io/event_dispatcher_kqueue.c | 17 +-------- src/server_lifecycle.c | 41 ++++++++++++++++++++++ src/server_lifecycle.h | 3 ++ tests/test_server_lifecycle.c | 55 ++++++++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 46 deletions(-) diff --git a/src/io/event_dispatcher_epoll.c b/src/io/event_dispatcher_epoll.c index 2ac2692..f564604 100644 --- a/src/io/event_dispatcher_epoll.c +++ b/src/io/event_dispatcher_epoll.c @@ -46,25 +46,11 @@ static void close_and_drop_client(const int epfd, client_t *c) if (!c) return; - if (server.verbose) { - printf("Dropping client fd=%d (%s:%d)\n", c->fd, c->ip_str, c->port); - } - struct epoll_event ev; memset(&ev, 0, sizeof(ev)); epoll_ctl(epfd, EPOLL_CTL_DEL, c->fd, &ev); - list_node_t *node = listFindNode(server.clients, NULL, c); - if (node) { - listDeleteNode(server.clients, node); - } - - close(c->fd); - server.num_clients -= 1; - server.num_disconnected_clients += 1; - update_disconnected_clients(&server.metrics, - server.num_disconnected_clients); - free_client(c); + server_drop_client(&server, c); } static int sync_client_write_interest(const int epfd, client_t *c) diff --git a/src/io/event_dispatcher_io_uring.c b/src/io/event_dispatcher_io_uring.c index 296fbc0..3d5d7b6 100644 --- a/src/io/event_dispatcher_io_uring.c +++ b/src/io/event_dispatcher_io_uring.c @@ -44,21 +44,7 @@ static void close_and_drop_client(struct io_uring *ring, client_t *c) if (!c) return; - if (server.verbose) { - printf("Dropping client fd=%d (%s:%d)\n", c->fd, c->ip_str, c->port); - } - - list_node_t *node = listFindNode(server.clients, NULL, c); - if (node) { - listDeleteNode(server.clients, node); - } - - close(c->fd); - server.num_clients -= 1; - server.num_disconnected_clients += 1; - update_disconnected_clients(&server.metrics, - server.num_disconnected_clients); - free_client(c); + server_drop_client(&server, c); } int run_event_loop() diff --git a/src/io/event_dispatcher_kqueue.c b/src/io/event_dispatcher_kqueue.c index dc5beba..ad96522 100644 --- a/src/io/event_dispatcher_kqueue.c +++ b/src/io/event_dispatcher_kqueue.c @@ -43,27 +43,12 @@ static void close_and_drop_client(const int kq, client_t *c) if (!c) return; - if (server.verbose) { - printf("Dropping client fd=%d (%s:%d)\n", c->fd, c->ip_str, c->port); - } - // deregister from kqueue struct kevent ch; EV_SET(&ch, c->fd, EVFILT_READ, EV_DELETE, 0, 0, NULL); (void)kevent(kq, &ch, 1, NULL, 0, NULL); - // remove from the server list - list_node_t *node = listFindNode(server.clients, NULL, c); - if (node) { - listDeleteNode(server.clients, node); - } - - close(c->fd); - server.num_clients -= 1; - server.num_disconnected_clients += 1; - update_disconnected_clients(&server.metrics, - server.num_disconnected_clients); - free_client(c); + server_drop_client(&server, c); } static int sync_client_write_interest(const int kq, client_t *c) diff --git a/src/server_lifecycle.c b/src/server_lifecycle.c index b202726..dea3f78 100644 --- a/src/server_lifecycle.c +++ b/src/server_lifecycle.c @@ -2,6 +2,8 @@ #include "client.h" #include +#include +#include #include #include @@ -43,6 +45,45 @@ static void close_clients(list_t *clients) listEmpty(clients); } +void server_drop_client(server_t *srv, client_t *client) +{ + if (!srv || !client) + return; + + if (srv->verbose) { + printf("Dropping client fd=%d (%s:%d)\n", client->fd, client->ip_str, + client->port); + } + + bool removed_from_list = false; + if (srv->clients) { + list_node_t *node = listFindNode(srv->clients, NULL, client); + if (node) { + void (*free_callback)(void *) = srv->clients->free; + srv->clients->free = NULL; + listDeleteNode(srv->clients, node); + srv->clients->free = free_callback; + removed_from_list = true; + } + } + + if (removed_from_list && srv->num_clients > 0) { + srv->num_clients -= 1; + } + + if (client->fd >= 0) { + close(client->fd); + client->fd = -1; + } + + if (srv->num_disconnected_clients < INT32_MAX) { + srv->num_disconnected_clients += 1; + } + srv->metrics.disconnected_clients = srv->num_disconnected_clients; + + free_client(client); +} + void shutdown_server(server_t *srv) { if (!srv) diff --git a/src/server_lifecycle.h b/src/server_lifecycle.h index 080171c..775c392 100644 --- a/src/server_lifecycle.h +++ b/src/server_lifecycle.h @@ -5,9 +5,12 @@ #include +typedef struct client_t client_t; + void request_server_shutdown(int sig); bool server_shutdown_requested(void); void reset_server_shutdown_request(void); +void server_drop_client(server_t *srv, client_t *client); void shutdown_server(server_t *srv); #endif // SERVER_LIFECYCLE_H diff --git a/tests/test_server_lifecycle.c b/tests/test_server_lifecycle.c index b0f967c..be7cdca 100644 --- a/tests/test_server_lifecycle.c +++ b/tests/test_server_lifecycle.c @@ -92,9 +92,64 @@ static void test_shutdown_server_releases_clients_and_database(void) printf("test_shutdown_server_releases_clients_and_database passed.\n"); } +static void test_server_drop_client_releases_tracked_client(void) +{ + int client_pair[2]; + assert(socketpair(AF_UNIX, SOCK_STREAM, 0, client_pair) == 0); + + server_t srv = {0}; + srv.clients = listCreate(); + assert(srv.clients != NULL); + + client_t *client = test_client(client_pair[0]); + srv.clients = listAddNodeToTail(srv.clients, client); + srv.num_clients = 1; + + server_drop_client(&srv, client); + + assert(srv.clients->len == 0); + assert(srv.num_clients == 0); + assert(srv.num_disconnected_clients == 1); + assert(srv.metrics.disconnected_clients == 1); + assert(fd_is_closed(client_pair[0])); + + free(srv.clients); + close(client_pair[1]); + + printf("test_server_drop_client_releases_tracked_client passed.\n"); +} + +static void test_server_drop_client_does_not_underflow_untracked_client(void) +{ + int client_pair[2]; + assert(socketpair(AF_UNIX, SOCK_STREAM, 0, client_pair) == 0); + + server_t srv = {0}; + srv.clients = listCreate(); + assert(srv.clients != NULL); + + client_t *client = test_client(client_pair[0]); + + server_drop_client(&srv, client); + + assert(srv.clients->len == 0); + assert(srv.num_clients == 0); + assert(srv.num_disconnected_clients == 1); + assert(srv.metrics.disconnected_clients == 1); + assert(fd_is_closed(client_pair[0])); + + free(srv.clients); + close(client_pair[1]); + + printf("test_server_drop_client_does_not_underflow_untracked_client " + "passed.\n"); +} + int main(void) { test_signal_handler_only_sets_shutdown_flag(); test_shutdown_server_releases_clients_and_database(); + test_server_drop_client_releases_tracked_client(); + test_server_drop_client_does_not_underflow_untracked_client(); return 0; }