RE: poll() emulation in git

From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
Sent: Wednesday, September 05, 2012 5:26 PM
To: Joachim Schmitz
Cc: 'Junio C Hamano'; git@vger.kernel.org; 'Erik Faye-Lund'; bug-gnulib@gnu.org
Subject: Re: poll() emulation in git

Il 05/09/2012 15:36, Joachim Schmitz ha scritto:

quoted

quoted

quoted

quoted

quoted

Does your system have a working FIONREAD ioctl for pipes?

It does have FIONREAD ioctl. Whether it works properly is to be determined...
I'll test if you could show me how?

Oh, now I see what you aimed at, but no, that Mac OS X method doesn't work for me, I tried (at least I think I did).

And <sys/ioctl.h> has
/*
 * Normal IOCTL's supported by the socket interface
 */
#define FIONREAD        _IOR(0, 8, _ioctl_int)       /* Num of bytes to read */
#define FIONBIO         _IOW(0, 9, _ioctl_int)       /* Non-blocking I/O     */

So these seem to be supported on sockets only, I guess.
And indeed the man pages for ioctl confirms:

          Valid values for the request parameter for AF_INET or
          AF_INET6 sockets are:


          FIONREAD  Gets the number of bytes available for reading and
                    stores it at the int pointed at by arg.


So not even AF_UNIX sockets, not to mention pipes...

So there's no way you can support POLLHUP.  Your system is quite
crippled. :(

Unfortunatly.

But is there something that could be done to make git work even without poll()?
It is used in 5 places:

$ grep -n poll\( *.c */*.c
credential-cache--daemon.c:175: if (poll(&pfd, 1, 1000 * wakeup) < 0) {
daemon.c:1018:          if (poll(pfd, socklist->nr, -1) < 0) {
help.c:361:                     poll(NULL, 0, autocorrect * 100);
upload-pack.c:232:              if (poll(pfd, pollsize, -1) < 0) {
builtin/upload-archive.c:125:           if (poll(pfd, 2, -1) < 0) {

Don't quite understand why in help.c it has that NULL, which should always result in an EFAULT and other than that basically is a
NOP (at least in the poll() emulation)? Seems a usleep(autocorrect * 100) is meant to happen here instead?
So I think here a poll() isn't needed at all. But also the 'broken' one shouldn't harm too much.

In daemon.c it seems to be all sockets it polls on, so it should work on NonStop.
Same in credential-cache--daemon.c

Remains upload-pack.c and builtin/upload-archive.c
In both start_command() gathers the FDs to poll() on and that indeed works on pipes -> problem on NonStop!

Seeing that in those cases xread() takes care of EAGAIN, I've now used 'brute force' in poll.c:

...
# else
      char data[64];
      r = recv (fd, data, sizeof (data), MSG_PEEK);
      socket_errno = (r < 0) ? errno : 0;
# endif
      if (r == 0)
        happened |= POLLHUP;

      /* If the event happened on an unconnected server socket,
         that's fine. */
      else if (r > 0 || ( /* (r == -1) && */ socket_errno == ENOTCONN))
        happened |= (POLLIN | POLLRDNORM) & sought;

      /* Distinguish hung-up sockets from other errors.  */
      else if (socket_errno == ESHUTDOWN || socket_errno == ECONNRESET
               || socket_errno == ECONNABORTED || socket_errno == ENETRESET)
        happened |= POLLHUP;

#ifdef __TANDEM /* as we can't recv(...,MSG_PEEK) on a non-socket */
      else if (socket_errno == ENOTSOCK)
        happened |= (POLLIN | POLLRDNORM) & sought;
#endif
      else
        happened |= POLLERR;
    }
...

We won't detect POLLHUP that way I think. However it seems to work, we've been able to clone, push, pull, branch that way with
NonStop being the (ssh-)server, something that didn't work at all without that hack (and yes, I believe it is just that).
Someone in for a cleaner way of managing this?

Bye, Jojo