[RFC/PATCH] core.clientTimeout config for git:// fetches

[RFC/PATCH] core.clientTimeout config for git:// fetches

[Eric Wong]

This allows "git fetch" to exit after a preconfigured socket
timeout.  This is needed in case an undetected server or network
failure occurs.

The default timeout is 60 seconds.

http:// and rsync:// transports should be supported in future
changes, too.

So far, I've only tested (by hand) with a dummy server setup
using "nc -l $some_port".
---

  I run "git fetch" from cron to keep mirrors up-to-date.  I've
  noticed some cron processes running for hours/days at a time after
  network failures.  When I straced thoses processes, I saw they were
  stuck on read() syscalls to remote machines.

  I'm not familiar with this code and hope there's a better way to do
  this.  Changing wrapper.c like this in this way doesn't feel right,
  but neither does sprinkling alarm() calls in various places.

  Using a custom GIT_PROXY_COMMAND which implements timeouts on its own
  would also work, but I'd like to eventually have a consistent parameter
  unified across all remote transports for consistency and ease-of-use.

  The default 60s timeout could be too low on some slow servers
  with big repos, maybe the default should remain zero (no timeout).

 connect.c |   27 +++++++++++++++++++++++++++
 wrapper.c |   47 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/connect.c b/connect.c
index ee1d4b4..c1fa1ab 100644
--- a/connect.c
+++ b/connect.c
@@ -320,10 +320,37 @@ static int git_tcp_connect_sock(char *host, int flags)
 
 #endif /* NO_IPV6 */
 
+static int timeout_setup(const char *var, const char *value, void *cb)
+{
+	int *timeout = cb;
+
+	if (strcmp(var, "core.clienttimeout") == 0)
+		*timeout = git_config_int(var, value);
+
+	return 0;
+}
 
 static void git_tcp_connect(int fd[2], char *host, int flags)
 {
 	int sockfd = git_tcp_connect_sock(host, flags);
+	struct timeval tv;
+	int timeout = 60;
+
+	git_config(timeout_setup, &timeout);
+
+	if (timeout > 0) {
+		socklen_t len = sizeof(struct timeval);
+
+		tv.tv_sec = timeout;
+		tv.tv_usec = 0;
+		if (setsockopt(sockfd, SOL_SOCKET, SO_RCVTIMEO, &tv, len) < 0)
+			die_errno("could not set socket receive timeout");
+
+		tv.tv_sec = timeout;
+		tv.tv_usec = 0;
+		if (setsockopt(sockfd, SOL_SOCKET, SO_SNDTIMEO, &tv, len) < 0)
+			die_errno("could not set socket send timeout");
+	}
 
 	fd[0] = sockfd;
 	fd[1] = dup(sockfd);
diff --git a/wrapper.c b/wrapper.c
index 85f09df..09a31c6 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -109,6 +109,49 @@ void *xcalloc(size_t nmemb, size_t size)
 	return ret;
 }
 
+static int io_continuable(int fd, int optname)
+{
+	if (errno == EINTR)
+		return 1;
+
+	/*
+	 * If fd is a socket, a socket-level timeout could've been
+	 * set in git_tcp_connect().  Unfortunately EAGAIN alone
+	 * isn't enough to tell us if the timeout is user-specified
+	 * or not, so we have to try to figure it out.
+	 */
+	if (errno == EAGAIN) {
+		socklen_t len = sizeof(struct timeval);
+		struct timeval tv;
+		int flags;
+
+		/*
+		 * Let any non-blocking I/O users through.  Currently
+		 * git does not explicitly use non-blocking I/O anywhere,
+		 * but it could inherit a non-blocking descriptor.  We
+		 * rely on the fact that (currently) no sockets git itself
+		 * creates (and thus may set timeout on) also sets O_NONBLOCK.
+		 */
+		flags = fcntl(fd, F_GETFL);
+		if (flags < 0)
+			die_errno("fcntl failed to get file status flags");
+		if (flags & O_NONBLOCK)
+			return 1;
+
+		if (getsockopt(fd, SOL_SOCKET, optname, &tv, &len) < 0) {
+			if (errno == ENOTSOCK)
+				return 1;
+			die_errno("getsockopt failed to get socket option");
+		}
+		if (tv.tv_sec > 0 || tv.tv_usec > 0)
+			die("socket timed out");
+
+		return 1;
+	}
+
+	return 0;
+}
+
 /*
  * xread() is the same a read(), but it automatically restarts read()
  * operations with a recoverable error (EAGAIN and EINTR). xread()
@@ -119,7 +162,7 @@ ssize_t xread(int fd, void *buf, size_t len)
 	ssize_t nr;
 	while (1) {
 		nr = read(fd, buf, len);
-		if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
+		if (nr < 0 && io_continuable(fd, SO_RCVTIMEO))
 			continue;
 		return nr;
 	}
@@ -135,7 +178,7 @@ ssize_t xwrite(int fd, const void *buf, size_t len)
 	ssize_t nr;
 	while (1) {
 		nr = write(fd, buf, len);
-		if ((nr < 0) && (errno == EAGAIN || errno == EINTR))
+		if (nr < 0 && io_continuable(fd, SO_SNDTIMEO))
 			continue;
 		return nr;
 	}
-- 
Eric Wong