From df0daf8ac0fce4bb98d5aff9295535a1606d2fad Mon Sep 17 00:00:00 2001
From: "Stephen R. van den Berg" <srb@cuci.nl>
Date: Thu, 14 Aug 2008 20:02:20 +0200
Subject: [PATCH 1/5] git-daemon: call logerror() instead of error()
Use logerror(), not error(), so that the messages won't be lost,
especially when running the daemon with its log sent to the syslog
facility.
Signed-off-by: Stephen R. van den Berg <srb@cuci.nl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
daemon.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/daemon.c b/daemon.c
index 8dcde73200d..93e11065382 100644
--- a/daemon.c
+++ b/daemon.c
@@ -836,7 +836,7 @@ static int socksetup(char *listen_addr, int listen_port, int **socklist_p)
if (sockfd < 0)
continue;
if (sockfd >= FD_SETSIZE) {
- error("too large socket descriptor.");
+ logerror("Socket descriptor too large");
close(sockfd);
continue;
}
@@ -955,7 +955,7 @@ static int service_loop(int socknum, int *socklist)
if (poll(pfd, socknum + 1, -1) < 0) {
if (errno != EINTR) {
- error("poll failed, resuming: %s",
+ logerror("Poll failed, resuming: %s",
strerror(errno));
sleep(1);
}
From 6a992e9e1da08af3e36ab90d5f5b7ee8077630ce Mon Sep 17 00:00:00 2001
From: "Stephen R. van den Berg" <srb@cuci.nl>
Date: Thu, 14 Aug 2008 20:02:20 +0200
Subject: [PATCH 2/5] git-daemon: use LOG_PID, simplify logging code
Make git-daemon use LOG_PID like most daemons, instead of prepending the
pid to the message ourselves, when using syslog(3).
Simplify the logging code by setting stderr to line buffered, instead of
building a single string and writing it out with a single write(2).
Give an extra log message at the daemon start-up.
Signed-off-by: Stephen R. van den Berg <srb@cuci.nl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
daemon.c | 47 ++++++++++++++++-------------------------------
1 file changed, 16 insertions(+), 31 deletions(-)
diff --git a/daemon.c b/daemon.c
index 93e11065382..3e663a877cb 100644
--- a/daemon.c
+++ b/daemon.c
@@ -78,38 +78,19 @@ static struct interp interp_table[] = {
static void logreport(int priority, const char *err, va_list params)
{
- /* We should do a single write so that it is atomic and output
- * of several processes do not get intermingled. */
- char buf[1024];
- int buflen;
- int maxlen, msglen;
-
- /* sizeof(buf) should be big enough for "[pid] \n" */
- buflen = snprintf(buf, sizeof(buf), "[%ld] ", (long) getpid());
-
- maxlen = sizeof(buf) - buflen - 1; /* -1 for our own LF */
- msglen = vsnprintf(buf + buflen, maxlen, err, params);
-
if (log_syslog) {
+ char buf[1024];
+ vsnprintf(buf, sizeof(buf), err, params);
syslog(priority, "%s", buf);
- return;
}
-
- /* maxlen counted our own LF but also counts space given to
- * vsnprintf for the terminating NUL. We want to make sure that
- * we have space for our own LF and NUL after the "meat" of the
- * message, so truncate it at maxlen - 1.
- */
- if (msglen > maxlen - 1)
- msglen = maxlen - 1;
- else if (msglen < 0)
- msglen = 0; /* Protect against weird return values. */
- buflen += msglen;
-
- buf[buflen++] = '\n';
- buf[buflen] = '\0';
-
- write_in_full(2, buf, buflen);
+ else {
+ /* Since stderr is set to linebuffered mode, the
+ * logging of different processes will not overlap
+ */
+ fprintf(stderr, "[%d] ", (int)getpid());
+ vfprintf(stderr, err, params);
+ fputc('\n', stderr);
+ }
}
static void logerror(const char *err, ...)
@@ -1178,9 +1159,11 @@ int main(int argc, char **argv)
}
if (log_syslog) {
- openlog("git-daemon", 0, LOG_DAEMON);
+ openlog("git-daemon", LOG_PID, LOG_DAEMON);
set_die_routine(daemon_die);
}
+ else
+ setlinebuf(stderr); /* avoid splitting a message in the middle */
if (inetd_mode && (group_name || user_name))
die("--user and --group are incompatible with --inetd");
@@ -1233,8 +1216,10 @@ int main(int argc, char **argv)
return execute(peer);
}
- if (detach)
+ if (detach) {
daemonize();
+ loginfo("Ready to rumble");
+ }
else
sanitize_stdfds();
From 695605b5080e1957bd9dab1fed35a7fee9814297 Mon Sep 17 00:00:00 2001
From: "Stephen R. van den Berg" <srb@cuci.nl>
Date: Thu, 14 Aug 2008 20:02:20 +0200
Subject: [PATCH 3/5] git-daemon: Simplify dead-children reaping logic
Move almost all code out of the child_handler() into check_dead_children().
The fact that systemcalls get interrupted by signals allows us to
make the SIGCHLD signal handler almost a no-op by simply running
check_dead_children() right before waiting on poll().
In case some systems do not interrupt systemcalls upon signal receipt,
all zombies will eventually be collected before the next poll() cycle.
Signed-off-by: Stephen R. van den Berg <srb@cuci.nl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
daemon.c | 55 +++++++++++++++++++++++--------------------------------
1 file changed, 23 insertions(+), 32 deletions(-)
diff --git a/daemon.c b/daemon.c
index 3e663a877cb..3abacfd9d92 100644
--- a/daemon.c
+++ b/daemon.c
@@ -16,7 +16,6 @@
static int log_syslog;
static int verbose;
static int reuseaddr;
-static int child_handler_pipe[2];
static const char daemon_usage[] =
"git daemon [--verbose] [--syslog] [--export-all]\n"
@@ -680,6 +679,21 @@ static void check_dead_children(void)
{
unsigned spawned, reaped, deleted;
+ for (;;) {
+ int status;
+ pid_t pid = waitpid(-1, &status, WNOHANG);
+
+ if (pid > 0) {
+ unsigned reaped = children_reaped;
+ if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
+ pid = -pid;
+ dead_child[reaped % MAX_CHILDREN] = pid;
+ children_reaped = reaped + 1;
+ continue;
+ }
+ break;
+ }
+
spawned = children_spawned;
reaped = children_reaped;
deleted = children_deleted;
@@ -760,21 +774,10 @@ static void handle(int incoming, struct sockaddr *addr, int addrlen)
static void child_handler(int signo)
{
- for (;;) {
- int status;
- pid_t pid = waitpid(-1, &status, WNOHANG);
-
- if (pid > 0) {
- unsigned reaped = children_reaped;
- if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
- pid = -pid;
- dead_child[reaped % MAX_CHILDREN] = pid;
- children_reaped = reaped + 1;
- write(child_handler_pipe[1], &status, 1);
- continue;
- }
- break;
- }
+ /* Otherwise empty handler because systemcalls will get interrupted
+ * upon signal receipt
+ * SysV needs the handler to be rearmed
+ */
signal(SIGCHLD, child_handler);
}
@@ -917,24 +920,21 @@ static int service_loop(int socknum, int *socklist)
struct pollfd *pfd;
int i;
- if (pipe(child_handler_pipe) < 0)
- die ("Could not set up pipe for child handler");
-
- pfd = xcalloc(socknum + 1, sizeof(struct pollfd));
+ pfd = xcalloc(socknum, sizeof(struct pollfd));
for (i = 0; i < socknum; i++) {
pfd[i].fd = socklist[i];
pfd[i].events = POLLIN;
}
- pfd[socknum].fd = child_handler_pipe[0];
- pfd[socknum].events = POLLIN;
signal(SIGCHLD, child_handler);
for (;;) {
int i;
- if (poll(pfd, socknum + 1, -1) < 0) {
+ check_dead_children();
+
+ if (poll(pfd, socknum, -1) < 0) {
if (errno != EINTR) {
logerror("Poll failed, resuming: %s",
strerror(errno));
@@ -942,10 +942,6 @@ static int service_loop(int socknum, int *socklist)
}
continue;
}
- if (pfd[socknum].revents & POLLIN) {
- read(child_handler_pipe[0], &i, 1);
- check_dead_children();
- }
for (i = 0; i < socknum; i++) {
if (pfd[i].revents & POLLIN) {
@@ -1036,11 +1032,6 @@ int main(int argc, char **argv)
gid_t gid = 0;
int i;
- /* Without this we cannot rely on waitpid() to tell
- * what happened to our children.
- */
- signal(SIGCHLD, SIG_DFL);
-
for (i = 1; i < argc; i++) {
char *arg = argv[i];
From 3bd62c21760f92996569bb9335b399a9545a5c41 Mon Sep 17 00:00:00 2001
From: "Stephen R. van den Berg" <srb@cuci.nl>
Date: Thu, 14 Aug 2008 20:02:20 +0200
Subject: [PATCH 4/5] git-daemon: rewrite kindergarden, new option
--max-connections
Get rid of the fixed array of children and make max-connections
dynamic and configurable.
Fix the killing code to actually kill the newest connections from
duplicate IP-addresses.
Avoid forking if too busy already.
Signed-off-by: Stephen R. van den Berg <srb@cuci.nl>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/git-daemon.txt | 9 +-
daemon.c | 217 +++++++++++++----------------------
2 files changed, 84 insertions(+), 142 deletions(-)
diff --git a/Documentation/git-daemon.txt b/Documentation/git-daemon.txt
index 4ba4b75c112..b08a08cd95b 100644
--- a/Documentation/git-daemon.txt
+++ b/Documentation/git-daemon.txt
@@ -9,8 +9,9 @@ SYNOPSIS
--------
[verse]
'git daemon' [--verbose] [--syslog] [--export-all]
- [--timeout=n] [--init-timeout=n] [--strict-paths]
- [--base-path=path] [--user-path | --user-path=path]
+ [--timeout=n] [--init-timeout=n] [--max-connections=n]
+ [--strict-paths] [--base-path=path] [--base-path-relaxed]
+ [--user-path | --user-path=path]
[--interpolated-path=pathtemplate]
[--reuseaddr] [--detach] [--pid-file=file]
[--enable=service] [--disable=service]
@@ -99,6 +100,10 @@ OPTIONS
it takes for the server to process the sub-request and time spent
waiting for next client's request.
+--max-connections::
+ Maximum number of concurrent clients, defaults to 32. Set it to
+ zero for no limit.
+
--syslog::
Log to syslog instead of stderr. Note that this option does not imply
--verbose, thus by default only error conditions will be logged.
diff --git a/daemon.c b/daemon.c
index 3abacfd9d92..79f03882048 100644
--- a/daemon.c
+++ b/daemon.c
@@ -19,8 +19,8 @@ static int reuseaddr;
static const char daemon_usage[] =
"git daemon [--verbose] [--syslog] [--export-all]\n"
-" [--timeout=n] [--init-timeout=n] [--strict-paths]\n"
-" [--base-path=path] [--base-path-relaxed]\n"
+" [--timeout=n] [--init-timeout=n] [--max-connections=n]\n"
+" [--strict-paths] [--base-path=path] [--base-path-relaxed]\n"
" [--user-path | --user-path=path]\n"
" [--interpolated-path=path]\n"
" [--reuseaddr] [--detach] [--pid-file=file]\n"
@@ -584,40 +584,35 @@ static int execute(struct sockaddr *addr)
return -1;
}
+static int max_connections = 32;
-/*
- * 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;
-static pid_t dead_child[MAX_CHILDREN];
-
-/* These are updated by the main loop */
-static unsigned int children_spawned;
-static unsigned int children_deleted;
+static unsigned int live_children;
static struct child {
+ struct child *next;
pid_t pid;
- int addrlen;
struct sockaddr_storage address;
-} live_child[MAX_CHILDREN];
+} *firstborn;
-static void add_child(int idx, pid_t pid, struct sockaddr *addr, int addrlen)
+static void add_child(pid_t pid, struct sockaddr *addr, int addrlen)
{
- live_child[idx].pid = pid;
- live_child[idx].addrlen = addrlen;
- memcpy(&live_child[idx].address, addr, addrlen);
+ struct child *newborn;
+ newborn = xcalloc(1, sizeof *newborn);
+ if (newborn) {
+ struct child **cradle;
+
+ live_children++;
+ newborn->pid = pid;
+ memcpy(&newborn->address, addr, addrlen);
+ for (cradle = &firstborn; *cradle; cradle = &(*cradle)->next)
+ if (!memcmp(&(*cradle)->address, &newborn->address,
+ sizeof newborn->address))
+ break;
+ newborn->next = *cradle;
+ *cradle = newborn;
+ }
+ else
+ logerror("Out of memory spawning new child");
}
/*
@@ -626,142 +621,78 @@ static void add_child(int idx, pid_t pid, struct sockaddr *addr, int addrlen)
* 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)
+static void remove_child(pid_t pid)
{
- struct child n;
+ struct child **cradle, *blanket;
- 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;
- }
+ for (cradle = &firstborn; (blanket = *cradle); cradle = &blanket->next)
+ if (blanket->pid == pid) {
+ *cradle = blanket->next;
+ live_children--;
+ free(blanket);
+ break;
+ }
}
/*
* 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.
+ * We kill the newest connection from a duplicate IP.
*/
-static void kill_some_children(int signo, unsigned start, unsigned stop)
+static void kill_some_child(void)
{
- start %= MAX_CHILDREN;
- stop %= MAX_CHILDREN;
- while (start != stop) {
- if (!(start & 3))
- kill(live_child[start].pid, signo);
- start = (start + 1) % MAX_CHILDREN;
+ const struct child *blanket;
+
+ if ((blanket = firstborn)) {
+ const struct child *next;
+
+ for (; (next = blanket->next); blanket = next)
+ if (!memcmp(&blanket->address, &next->address,
+ sizeof next->address)) {
+ kill(blanket->pid, SIGTERM);
+ break;
+ }
}
}
static void check_dead_children(void)
{
- unsigned spawned, reaped, deleted;
+ int status;
+ pid_t pid;
- for (;;) {
- int status;
- pid_t pid = waitpid(-1, &status, WNOHANG);
-
- if (pid > 0) {
- unsigned reaped = children_reaped;
- if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
- pid = -pid;
- dead_child[reaped % MAX_CHILDREN] = pid;
- children_reaped = reaped + 1;
- continue;
- }
- break;
- }
-
- spawned = children_spawned;
- reaped = children_reaped;
- deleted = children_deleted;
-
- while (deleted < reaped) {
- pid_t pid = dead_child[deleted % MAX_CHILDREN];
- const char *dead = pid < 0 ? " (with error)" : "";
-
- if (pid < 0)
- pid = -pid;
-
- /* XXX: Custom logging, since we don't wanna getpid() */
- if (verbose) {
- if (log_syslog)
- syslog(LOG_INFO, "[%d] Disconnected%s",
- pid, dead);
- else
- fprintf(stderr, "[%d] Disconnected%s\n",
- pid, dead);
- }
- remove_child(pid, deleted, spawned);
- deleted++;
- }
- children_deleted = deleted;
-}
-
-static void check_max_connections(void)
-{
- for (;;) {
- int active;
- unsigned spawned, deleted;
-
- check_dead_children();
-
- spawned = children_spawned;
- deleted = children_deleted;
-
- active = spawned - deleted;
- if (active <= max_connections)
- break;
-
- /* Kill some unstarted connections with SIGTERM */
- kill_some_children(SIGTERM, deleted, spawned);
- if (active <= max_connections << 1)
- break;
-
- /* If the SIGTERM thing isn't helping use SIGKILL */
- kill_some_children(SIGKILL, deleted, spawned);
- sleep(1);
+ while ((pid = waitpid(-1, &status, WNOHANG))>0) {
+ const char *dead = "";
+ remove_child(pid);
+ if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
+ dead = " (with error)";
+ loginfo("[%d] Disconnected%s", (int)pid, dead);
}
}
static void handle(int incoming, struct sockaddr *addr, int addrlen)
{
- pid_t pid = fork();
+ pid_t pid;
- if (pid) {
- unsigned idx;
-
- close(incoming);
- if (pid < 0)
+ if (max_connections && live_children >= max_connections) {
+ kill_some_child();
+ sleep(1); /* give it some time to die */
+ check_dead_children();
+ if (live_children >= max_connections) {
+ close(incoming);
+ logerror("Too many children, dropping connection");
return;
+ }
+ }
- idx = children_spawned % MAX_CHILDREN;
- children_spawned++;
- add_child(idx, pid, addr, addrlen);
+ if ((pid = fork())) {
+ close(incoming);
+ if (pid < 0) {
+ logerror("Couldn't fork %s", strerror(errno));
+ return;
+ }
- check_max_connections();
+ add_child(pid, addr, addrlen);
return;
}
@@ -1077,6 +1008,12 @@ int main(int argc, char **argv)
init_timeout = atoi(arg+15);
continue;
}
+ if (!prefixcmp(arg, "--max-connections=")) {
+ max_connections = atoi(arg+18);
+ if (max_connections < 0)
+ max_connections = 0; /* unlimited */
+ continue;
+ }
if (!strcmp(arg, "--strict-paths")) {
strict_paths = 1;
continue;
From 460c201039471d22194ca871290c098bfe6ce6a3 Mon Sep 17 00:00:00 2001
From: Junio C Hamano <gitster@pobox.com>
Date: Sun, 24 Aug 2008 13:27:10 -0700
Subject: [PATCH 5/5] daemon.c: minor style fixup
* "else" on the same line as "}" that closes corresponding "if (...) {";
* multi-line comments begin with "/*\n";
* sizeof, even it is not a function, is written as "sizeof(...)";
* no need to check x?alloc() return value -- it would have died;
* "if (...) { ... }" that covers the whole function body can be dedented
by returning from the function early with "if (!...) return;";
* SP on each side of an operator, i.e. "a > 0", not "a>0";
Also removes stale comment describing how remove_child() used to do its
thing.
Signed-off-by: Junio C Hamano <gitster@pobox.com>:
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
daemon.c | 74 +++++++++++++++++++++++++-------------------------------
1 file changed, 33 insertions(+), 41 deletions(-)
diff --git a/daemon.c b/daemon.c
index 79f03882048..23278e28dc1 100644
--- a/daemon.c
+++ b/daemon.c
@@ -81,9 +81,9 @@ static void logreport(int priority, const char *err, va_list params)
char buf[1024];
vsnprintf(buf, sizeof(buf), err, params);
syslog(priority, "%s", buf);
- }
- else {
- /* Since stderr is set to linebuffered mode, the
+ } else {
+ /*
+ * Since stderr is set to linebuffered mode, the
* logging of different processes will not overlap
*/
fprintf(stderr, "[%d] ", (int)getpid());
@@ -596,31 +596,24 @@ static struct child {
static void add_child(pid_t pid, struct sockaddr *addr, int addrlen)
{
- struct child *newborn;
- newborn = xcalloc(1, sizeof *newborn);
- if (newborn) {
- struct child **cradle;
+ struct child *newborn, **cradle;
- live_children++;
- newborn->pid = pid;
- memcpy(&newborn->address, addr, addrlen);
- for (cradle = &firstborn; *cradle; cradle = &(*cradle)->next)
- if (!memcmp(&(*cradle)->address, &newborn->address,
- sizeof newborn->address))
- break;
- newborn->next = *cradle;
- *cradle = newborn;
- }
- else
- logerror("Out of memory spawning new child");
+ /*
+ * This must be xcalloc() -- we'll compare the whole sockaddr_storage
+ * but individual address may be shorter.
+ */
+ newborn = xcalloc(1, sizeof(*newborn));
+ live_children++;
+ newborn->pid = pid;
+ memcpy(&newborn->address, addr, addrlen);
+ for (cradle = &firstborn; *cradle; cradle = &(*cradle)->next)
+ if (!memcmp(&(*cradle)->address, &newborn->address,
+ sizeof(newborn->address)))
+ break;
+ newborn->next = *cradle;
+ *cradle = newborn;
}
-/*
- * 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)
{
struct child **cradle, *blanket;
@@ -642,18 +635,17 @@ static void remove_child(pid_t pid)
*/
static void kill_some_child(void)
{
- const struct child *blanket;
+ const struct child *blanket, *next;
- if ((blanket = firstborn)) {
- const struct child *next;
+ if (!(blanket = firstborn))
+ return;
- for (; (next = blanket->next); blanket = next)
- if (!memcmp(&blanket->address, &next->address,
- sizeof next->address)) {
- kill(blanket->pid, SIGTERM);
- break;
- }
- }
+ for (; (next = blanket->next); blanket = next)
+ if (!memcmp(&blanket->address, &next->address,
+ sizeof(next->address))) {
+ kill(blanket->pid, SIGTERM);
+ break;
+ }
}
static void check_dead_children(void)
@@ -661,10 +653,10 @@ static void check_dead_children(void)
int status;
pid_t pid;
- while ((pid = waitpid(-1, &status, WNOHANG))>0) {
+ while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
const char *dead = "";
remove_child(pid);
- if (!WIFEXITED(status) || WEXITSTATUS(status) > 0)
+ if (!WIFEXITED(status) || (WEXITSTATUS(status) > 0))
dead = " (with error)";
loginfo("[%d] Disconnected%s", (int)pid, dead);
}
@@ -676,7 +668,7 @@ static void handle(int incoming, struct sockaddr *addr, int addrlen)
if (max_connections && live_children >= max_connections) {
kill_some_child();
- sleep(1); /* give it some time to die */
+ sleep(1); /* give it some time to die */
check_dead_children();
if (live_children >= max_connections) {
close(incoming);
@@ -705,7 +697,8 @@ static void handle(int incoming, struct sockaddr *addr, int addrlen)
static void child_handler(int signo)
{
- /* Otherwise empty handler because systemcalls will get interrupted
+ /*
+ * Otherwise empty handler because systemcalls will get interrupted
* upon signal receipt
* SysV needs the handler to be rearmed
*/
@@ -1089,8 +1082,7 @@ int main(int argc, char **argv)
if (log_syslog) {
openlog("git-daemon", LOG_PID, LOG_DAEMON);
set_die_routine(daemon_die);
- }
- else
+ } else
setlinebuf(stderr); /* avoid splitting a message in the middle */
if (inetd_mode && (group_name || user_name))