git-daemon: actually remember the children we have outstanding

This is using a lockless approach that allows us to handle children
dying without having to block SIGCHLD.

Right now our "solution" to too many kids is pretty damn rough, but it
at least shows what you can do.

diff --git a/daemon.c b/daemon.c
index e6fbab6..3219615 100644 (file)
--- a/daemon.c
+++ b/daemon.c
@@ -4,21 +4,10 @@
 #include <sys/wait.h>
 #include <sys/socket.h>
 #include <netinet/in.h>
+#include <arpa/inet.h>
 
 static const char daemon_usage[] = "git-daemon [--inetd | --port=n]";
 
-/* We don't actually do anything about this yet */
-static int max_connections = 10;
-
-/*
- * We count spawned/reaped separately, just to avoid any
- * races when updating them from signals. The SIGCHLD handler
- * will only update children_reaped, and the fork logic will
- * only update children_spawned.
- */
-static unsigned int children_spawned = 0;
-static unsigned int children_reaped = 0;
-
 static int upload(char *dir, int dirlen)
 {
        if (chdir(dir) < 0)
@@ -59,26 +48,133 @@ static int execute(void)
        return -1;
 }
 
+
+/*
+ * We count spawned/reaped separately, just to avoid any
+ * races when updating them from signals. The SIGCHLD handler
+ * will only update children_reaped, and the fork logic will
+ * only update children_spawned.
+ *
+ * MAX_CHILDREN should be a power-of-two to make the modulus
+ * operation cheap. It should also be at least twice
+ * the maximum number of connections we will ever allow.
+ */
+#define MAX_CHILDREN 128
+
+static int max_connections = 25;
+
+/* These are updated by the signal handler */
+static volatile unsigned int children_reaped = 0;
+pid_t dead_child[MAX_CHILDREN];
+
+/* These are updated by the main loop */
+static unsigned int children_spawned = 0;
+static unsigned int children_deleted = 0;
+
+struct child {
+       pid_t pid;
+       int addrlen;
+       struct sockaddr_in address;
+} live_child[MAX_CHILDREN];
+
+static void add_child(int idx, pid_t pid, struct sockaddr_in *addr, int addrlen)
+{
+       live_child[idx].pid = pid;
+       live_child[idx].addrlen = addrlen;
+       live_child[idx].address = *addr;
+}
+
+/*
+ * Walk from "deleted" to "spawned", and remove child "pid".
+ *
+ * We move everything up by one, since the new "deleted" will
+ * be one higher.
+ */
+static void remove_child(pid_t pid, unsigned deleted, unsigned spawned)
+{
+       struct child n;
+
+       deleted %= MAX_CHILDREN;
+       spawned %= MAX_CHILDREN;
+       if (live_child[deleted].pid == pid) {
+               live_child[deleted].pid = -1;
+               return;
+       }
+       n = live_child[deleted];
+       for (;;) {
+               struct child m;
+               deleted = (deleted + 1) % MAX_CHILDREN;
+               if (deleted == spawned)
+                       die("could not find dead child %d\n", pid);
+               m = live_child[deleted];
+               live_child[deleted] = n;
+               if (m.pid == pid)
+                       return;
+               n = m;
+       }
+}
+
+/*
+ * This gets called if the number of connections grows
+ * past "max_connections".
+ *
+ * We _should_ start off by searching for connections
+ * from the same IP, and if there is some address wth
+ * multiple connections, we should kill that first.
+ *
+ * As it is, we just "randomly" kill 25% of the connections,
+ * and our pseudo-random generator sucks too. I have no
+ * shame.
+ *
+ * Really, this is just a place-holder for a _real_ algorithm.
+ */
+static void kill_some_children(int connections, unsigned start, unsigned stop)
+{
+       start %= MAX_CHILDREN;
+       stop %= MAX_CHILDREN;
+       while (start != stop) {
+               if (!(start & 3))
+                       kill(live_child[start].pid, SIGTERM);
+               start = (start + 1) % MAX_CHILDREN;
+       }
+}
+
 static void handle(int incoming, struct sockaddr_in *addr, int addrlen)
 {
        pid_t pid = fork();
 
        if (pid) {
                int active;
+               unsigned spawned, reaped, deleted;
 
                close(incoming);
                if (pid < 0)
                        return;
 
-               active = ++children_spawned - children_reaped;
+               spawned = children_spawned;
+               add_child(spawned % MAX_CHILDREN, pid, addr, addrlen);
+               children_spawned = ++spawned;
+
+               reaped = children_reaped;
+               deleted = children_deleted;
+
+               while (deleted < reaped) {
+                       pid_t pid = dead_child[deleted % MAX_CHILDREN];
+                       remove_child(pid, deleted, spawned);
+                       deleted++;
+               }
+               children_deleted = deleted;
+
+               active = spawned - deleted;
                if (active > max_connections) {
-                       /*
-                        * Fixme! This is where you'd have to do something to
-                        * limit the number of children. Like killing off random
-                        * ones, or at least the ones that haven't even gotten
-                        * started yet.
-                        */
+                       kill_some_children(active, deleted, spawned);
+
+                       /* Wait to make sure they're gone */
+                       while (spawned - children_reaped > max_connections)
+                               sleep(1);
                }
+                       
+
                return;
        }
 
@@ -91,8 +187,12 @@ static void handle(int incoming, struct sockaddr_in *addr, int addrlen)
 static void child_handler(int signo)
 {
        for (;;) {
-               if (waitpid(-1, NULL, WNOHANG) > 0) {
-                       children_reaped++;
+               pid_t pid = waitpid(-1, NULL, WNOHANG);
+
+               if (pid > 0) {
+                       unsigned reaped = children_reaped;
+                       dead_child[reaped % MAX_CHILDREN] = pid;
+                       children_reaped = reaped + 1;
                        continue;
                }
                break;