Re: git-daemon problem

Matthias Lederhofer [off-list ref] writes:

A few weeks ago upgrading from 1.3.x to 1.4.1 I had a problem with
git-daemon.  I started git-daemon on a terminal but did not redirect
stdin/stdout/stderr to /dev/null (actually using daemon(8) on freebsd
without -f but just disowning the process and closing the terminal
works fine too, nothing freebsd/daemon(8) specific).  After closing
the terminal I was not able to use the git-daemon anymore with some
versions of the git. So now I took some time and tried to find what
was the reason for that.

HMMMMMMMMM.

I did this silly experiment.  Close, instead of connecting them
to /dev/null, the low 3 file descriptors:

$ git-daemon 0<&- 1>&- 2>&- --export-all /pub/git &
$ lsof -p $! | sed -n -e '/ FD /p' -e '/[0-9]u/p'
COMMAND     PID  USER   FD   TYPE  DEVICE    SIZE    NODE NAME
git-daemo 13921 junio    0u  IPv6 1973673             TCP *:9419 (LISTEN)
git-daemo 13921 junio    1u  IPv4 1973674             TCP *:9419 (LISTEN)

It gets worse.  This is what happens when you close only fd #2.

$ git-daemon 2>&- --export-all /pub/git &
$ lsof -p $! | sed -n -e '/ FD /p' -e '/[0-9]u/p'
COMMAND     PID  USER   FD   TYPE  DEVICE    SIZE    NODE NAME
git-daemo 13961 junio    0u   CHR   136,7               9 /dev/pts/7
git-daemo 13961 junio    1u   CHR   136,7               9 /dev/pts/7
git-daemo 13961 junio    2u  IPv6 1975187             TCP *:9419 (LISTEN)
git-daemo 13961 junio    3u  IPv4 1975188             TCP *:9419 (LISTEN)

Now, after we do accept(), we spawn a subprocess in handle(),
and in the child process dup2() the fd connected to the peer to
fd 0 and 1 of the child process -- and we do not do anything to
fd 2 of the child process.

If you do not close any of the low 3 file descriptors, fd 2 of
the child process is connected to whatever error stream of
daemon is, so you would not see this problem, but this certainly
is bad.

Maybe we should check if fd 2 is sane at daemon startup, and
otherwise open /dev/null for writing and dup2 it to fd 2?

Currently, under --inetd mode we have freopen of stderr, but
that does not help this issue.  It would make die() and error()
in daemon itself behave sanely but when you start the daemon
with the low file descriptors closed, fileno(stderr) may be
different from 2.

Junio C Hamano [off-list ref] wrote:

This breaks the newer clients that knows how to do side-band
doesn't it?

Probably, this patch is just to give a starting point where the
problem could be.

Would this replacement patch help?

[use write instead of safe_write]

This seems to fix it.  Perhaps it should be xwrite instead of write.

Maybe we should check if fd 2 is sane at daemon startup, and
otherwise open /dev/null for writing and dup2 it to fd 2?

daemon startup is probably not the right place because as long as the
terminal is open this will be fine.