From 8c537229d103d64f93afd205be6fad420ca5fdc0 Mon Sep 17 00:00:00 2001 From: Hamish Coleman Date: Thu, 9 May 2024 16:19:12 +1000 Subject: [PATCH] Be more careful with selected file descriptors Some bug reports that edges using TCP mode are getting occasional errors: *** bit out of range 0 - FD_SETSIZE on fd_set ***: terminated We were not accounting for the fact that the TCP supernode socket file descriptor is set to "-1" when it is disconnected and invalid. Avoid "-1" file handles when adding to or checking signals for fd_set contents. This code path will hopefully also get smoothed out when the mainloop conversion is added in - as it will have a systematic way to add/check file descriptors. (Addresses #28) --- src/edge_utils.c | 41 ++++++++++++++++++++++++----------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/edge_utils.c b/src/edge_utils.c index 81b83588..4f90b80d 100644 --- a/src/edge_utils.c +++ b/src/edge_utils.c @@ -1027,21 +1027,28 @@ static void check_known_peer_sock_change (struct n3n_runtime_data *eee, * TODO: for the TCP case, this could cause a stall in the packet * send path, so this probably should be reworked to use a queue */ -static int check_sock_ready (struct n3n_runtime_data *eee) { - // if required (tcp), wait until writeable as soket is set to - // O_NONBLOCK, could require some wait time directly after re-opening - if(eee->conf.connect_tcp) { - fd_set socket_mask; - struct timeval wait_time; +static bool check_sock_ready (struct n3n_runtime_data *eee) { + if(!eee->conf.connect_tcp) { + // Just show udp sockets as ready + // TODO: this is may not be always true + return true; + } - FD_ZERO(&socket_mask); - FD_SET(eee->sock, &socket_mask); - wait_time.tv_sec = 0; - wait_time.tv_usec = 500000; - return select(eee->sock + 1, NULL, &socket_mask, NULL, &wait_time); + if(eee->sock == -1) { + // If we have no sock, dont attempt to FD_SET() it + return false; } - return 1; + // if required (tcp), wait until writeable as soket is set to + // O_NONBLOCK, could require some wait time directly after re-opening + fd_set socket_mask; + struct timeval wait_time; + + FD_ZERO(&socket_mask); + FD_SET(eee->sock, &socket_mask); + wait_time.tv_sec = 0; + wait_time.tv_usec = 500000; + return select(eee->sock + 1, NULL, &socket_mask, NULL, &wait_time); } /** Send a datagram to a socket file descriptor */ @@ -1051,7 +1058,7 @@ static ssize_t sendto_fd (struct n3n_runtime_data *eee, const void *buf, ssize_t sent = 0; - if(check_sock_ready(eee) < 1) { + if(!check_sock_ready(eee)) { goto err_out; } @@ -3024,8 +3031,8 @@ int run_edge_loop (struct n3n_runtime_data *eee) { if(rc > 0) { // any or all of the FDs could have input; check them all - // external - if(FD_ISSET(eee->sock, &readers)) { + // external packets + if((eee->sock != -1) && FD_ISSET(eee->sock, &readers)) { if(0 != fetch_and_eventually_process_data( eee, eee->sock, @@ -3050,7 +3057,7 @@ int run_edge_loop (struct n3n_runtime_data *eee) { } #ifndef SKIP_MULTICAST_PEERS_DISCOVERY - if(FD_ISSET(eee->udp_multicast_sock, &readers)) { + if((eee->udp_multicast_sock != -1) && FD_ISSET(eee->udp_multicast_sock, &readers)) { if(0 != fetch_and_eventually_process_data( eee, eee->udp_multicast_sock, @@ -3065,7 +3072,7 @@ int run_edge_loop (struct n3n_runtime_data *eee) { #endif #ifndef _WIN32 - if(FD_ISSET(eee->device.fd, &readers)) { + if((eee->device.fd != -1) && FD_ISSET(eee->device.fd, &readers)) { // read an ethernet frame from the TAP socket; write on the IP socket edge_read_from_tap(eee); }