diff --git a/dump1090.c b/dump1090.c index d93cddb..0293ca7 100644 --- a/dump1090.c +++ b/dump1090.c @@ -582,6 +582,7 @@ void backgroundTasks(void) { if (Modes.net) { modesReadFromClients(); + modesNetCleanup(); } // If Modes.aircrafts is not NULL, remove any stale aircraft diff --git a/dump1090.h b/dump1090.h index babea25..32a8fc4 100644 --- a/dump1090.h +++ b/dump1090.h @@ -459,6 +459,7 @@ void modesReadFromClients (void); void modesSendAllClients (int service, void *msg, int len); void modesQueueOutput (struct modesMessage *mm); void modesReadFromClient(struct client *c, char *sep, int(*handler)(struct client *, char *)); +void modesNetCleanup (void); #ifdef __cplusplus } diff --git a/net_io.c b/net_io.c index f3a8397..b092c33 100644 --- a/net_io.c +++ b/net_io.c @@ -145,23 +145,11 @@ struct client * modesAcceptClients(void) { // On error free the client, collect the structure, adjust maxfd if needed. // void modesFreeClient(struct client *c) { + // Clean up, but defer removing from the list until modesNetCleanup(). + // This is because there may be stackframes still pointing at this + // client (unpredictably: reading from client A may cause client B to + // be freed) - // Unhook this client from the linked list of clients - struct client *p = Modes.clients; - if (p) { - if (p == c) { - Modes.clients = c->next; - } else { - while ((p) && (p->next != c)) { - p = p->next; - } - if (p) { - p->next = c->next; - } - } - } - - // It's now safe to remove this client close(c->fd); if (c->service == Modes.sbsos) { if (Modes.stat_sbs_connections) Modes.stat_sbs_connections--; @@ -174,7 +162,9 @@ void modesFreeClient(struct client *c) { if (Modes.debug & MODES_DEBUG_NET) printf("Closing client %d\n", c->fd); - free(c); + // mark it as inactive and ready to be freed + c->fd = -1; + c->service = -1; } // //========================================================================= @@ -182,12 +172,9 @@ void modesFreeClient(struct client *c) { // Send the specified message to all clients listening for a given service // void modesSendAllClients(int service, void *msg, int len) { - struct client *c = Modes.clients; - - while (c) { - // Read next before servicing client incase the service routine deletes the client! - struct client *next = c->next; + struct client *c; + for (c = Modes.clients; c; c = c->next) { if (c->service == service) { #ifndef _WIN32 int nwritten = write(c->fd, msg, len); @@ -198,7 +185,6 @@ void modesSendAllClients(int service, void *msg, int len) { modesFreeClient(c); } } - c = next; } } // @@ -952,10 +938,7 @@ void modesReadFromClients(void) { struct client *c = modesAcceptClients(); - while (c) { - // Read next before servicing client incase the service routine deletes the client! - struct client *next = c->next; - + for (; c; c = c->next) { if (c->service == Modes.ris) { modesReadFromClient(c,"\n",decodeHexMessage); } else if (c->service == Modes.bis) { @@ -963,9 +946,26 @@ void modesReadFromClients(void) { } else if (c->service == Modes.https) { modesReadFromClient(c,"\r\n\r\n",handleHTTPRequest); } - c = next; } } + +// +// Perform cleanup of any recently-closed clients. +// +void modesNetCleanup(void) { + struct client *c, **prev; + + for (prev = &Modes.clients, c = *prev; c; c = *prev) { + if (c->fd == -1) { + // Recently closed, prune from list + *prev = c->next; + free(c); + } else { + prev = &c->next; + } + } + } + // // =============================== Network IO =========================== //