fix: centralize client drop cleanup

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)

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()

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)

src/server_lifecycle.c

@@ -2,6 +2,8 @@
 #include "client.h"
 
 #include <signal.h>
+#include <stdint.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <unistd.h>
 
@@ -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)

src/server_lifecycle.h

@@ -5,9 +5,12 @@
 
 #include <stdbool.h>
 
+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

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;
 }