Fix a crash when an outbound client is closed in response to reading a remote message.

When we read from some client A, we may end up forwarding a message to other
clients. If we forward to some client B and there is a write error, then
we close B and remove it from the client list. However, if before this happened
A->next == B, then the read loop will still be holding on to a pointer to B,
and we crash.

As it's unpredictable what clients could be closed at what point, the simplest
approach is to retain closed clients in the list until we are at a point where
we know there are no stray pointers on stack, and only then modify the list.
This also simplifies anything that has to loop over clients, as it doesn't need
to worry about the current client being freed under it.

dump1090.c

@@ -490,6 +490,7 @@ void showCopyright(void) {
 void backgroundTasks(void) {
     if (Modes.net) {
         modesReadFromClients();
+        modesNetCleanup();
     }    
 
     // If Modes.aircrafts is not NULL, remove any stale aircraft

dump1090.h

@@ -454,6 +454,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
 }

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;
     }
 }
 //
@@ -944,10 +930,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) {
@@ -955,9 +938,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 ===========================
 //