{"thread":{"id":"42446","subject":"[PATCH] daemon: enable SO_KEEPALIVE for all sockets","startedAt":"2016-05-25T03:15:05Z","lastAt":"2016-05-25T23:11:14Z","messageCount":4,"participants":["Eric Wong","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"287474","messageId":"20160525031505.5720-1-e@80x24.org","threadId":"42446","inReplyTo":null,"subject":"[PATCH] daemon: enable SO_KEEPALIVE for all sockets","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-25T03:15:05Z","receivedAt":"2016-05-25T03:15:05Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"While --init-timeout and --timeout options exist and I've never\nrun git-daemon without them, some users may forget to set them\nand encounter hung daemon processes when connections fail.\nEnable socket-level timeouts so the kernel can send keepalive\nprobes as necessary to detect failed connections.\n\nSigned-off-by: Eric Wong \n---\n daemon.c | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n\ndiff --git a/daemon.c b/daemon.c\nindex 8d45c33..46dddac 100644\n--- a/daemon.c\n+++ b/daemon.c\n@@ -669,6 +669,15 @@ static void hostinfo_clear(struct hostinfo *hi)\n \tstrbuf_release(&hi->tcp_port);\n }\n \n+static void set_keep_alive(int sockfd)\n+{\n+\tint ka = 1;\n+\n+\tif (setsockopt(sockfd, SOL_SOCKET, SO_KEEPALIVE, &ka, sizeof(ka)) < 0)\n+\t\tlogerror(\"unable to set SO_KEEPALIVE on socket: %s\",\n+\t\t\tstrerror(errno));\n+}\n+\n static int execute(void)\n {\n \tchar *line = packet_buffer;\n@@ -681,6 +690,7 @@ static int execute(void)\n \tif (addr)\n \t\tloginfo(\"Connection from %s:%s\", addr, port);\n \n+\tset_keep_alive(0);\n \talarm(init_timeout ? init_timeout : timeout);\n \tpktlen = packet_read(0, NULL, NULL, packet_buffer, sizeof(packet_buffer), 0);\n \talarm(0);\n@@ -951,6 +961,8 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis\n \t\t\tcontinue;\n \t\t}\n \n+\t\tset_keep_alive(sockfd);\n+\n \t\tif (bind(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {\n \t\t\tlogerror(\"Could not bind to %s: %s\",\n \t\t\t\t ip2str(ai->ai_family, ai->ai_addr, ai->ai_addrlen),\n@@ -1010,6 +1022,8 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis\n \t\treturn 0;\n \t}\n \n+\tset_keep_alive(sockfd);\n+\n \tif ( bind(sockfd, (struct sockaddr *)&sin, sizeof sin) < 0 ) {\n \t\tlogerror(\"Could not bind to %s: %s\",\n \t\t\t ip2str(AF_INET, (struct sockaddr *)&sin, sizeof(sin)),\n"},{"id":"287524","messageId":"20160525224235.GE13776@sigill.intra.peff.net","threadId":"42446","inReplyTo":"20160525031505.5720-1-e@80x24.org","subject":"Re: [PATCH] daemon: enable SO_KEEPALIVE for all sockets","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-05-25T22:42:36Z","receivedAt":"2016-05-25T22:42:36Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 25, 2016 at 03:15:05AM +0000, Eric Wong wrote:\n\n> While --init-timeout and --timeout options exist and I've never\n> run git-daemon without them, some users may forget to set them\n> and encounter hung daemon processes when connections fail.\n> Enable socket-level timeouts so the kernel can send keepalive\n> probes as necessary to detect failed connections.\n\nMakes sense. I wondered if there were any portability issues here, but\nit looks like the same code is found on the client side (but we'd still\nwant it here for cases where the client thinks the connection is dead\nbut the server does not realize it).\n\n-Peff\n"},{"id":"287525","messageId":"xmqq37p5zrjk.fsf@gitster.mtv.corp.google.com","threadId":"42446","inReplyTo":"20160525031505.5720-1-e@80x24.org","subject":"Re: [PATCH] daemon: enable SO_KEEPALIVE for all sockets","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-05-25T22:44:31Z","receivedAt":"2016-05-25T22:44:31Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Wong writes:\n\n> While --init-timeout and --timeout options exist and I've never\n> run git-daemon without them, some users may forget to set them\n> and encounter hung daemon processes when connections fail.\n> Enable socket-level timeouts so the kernel can send keepalive\n> probes as necessary to detect failed connections.\n>\n> Signed-off-by: Eric Wong \n> ---\n\nThis makes sense as a follow-up to e47a8583 (enable SO_KEEPALIVE for\nconnected TCP sockets, 2011-12-06), I think.\n"},{"id":"287535","messageId":"20160525231114.GA20691@dcvr.yhbt.net","threadId":"42446","inReplyTo":"xmqq37p5zrjk.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] daemon: enable SO_KEEPALIVE for all sockets","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2016-05-25T23:11:14Z","receivedAt":"2016-05-25T23:11:14Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Junio C Hamano wrote:\n> This makes sense as a follow-up to e47a8583 (enable SO_KEEPALIVE for\n> connected TCP sockets, 2011-12-06), I think.\n\nYes, a15d069a19867 for http, too; hat trick :>\n\nAnyways, it might've helped Savannah when they had networking\nproblems:\n\n http://mid.gmane.org/20160524214102858920068@bob.proulx.com\n\nThey might be running an old version that didn't send keepalive\nheartbeats during packing, too. But SO_KEEPALIVE will still\nhelp during init when --init-timeout is not set.\n\nPerhaps it also makes sense to squash the following xinetd\nsetting into giteveryday.txt, too; since some users could be\nrunning out-of-date git but reading new documentation on\nthe web:\n\n--- a/Documentation/giteveryday.txt\n+++ b/Documentation/giteveryday.txt\n@@ -390,6 +390,7 @@ service git\n \tserver = /usr/bin/git-daemon\n \tserver_args = --inetd --export-all --base-path=/pub/scm\n \tlog_on_failure += USERID\n+\tflags = KEEPALIVE\n }\n ------------\n +\n"}]}