RE: poll() emulation in git

[Joachim Schmitz]

> From: Paolo Bonzini [mailto:address@hidden On Behalf Of Paolo Bonzini
> Sent: Wednesday, September 05, 2012 5:26 PM
> To: Joachim Schmitz
> Cc: 'Junio C Hamano'; address@hidden; 'Erik Faye-Lund'; address@hidden
> Subject: Re: poll() emulation in git
> 
> Il 05/09/2012 15:36, Joachim Schmitz ha scritto:
> >>> > > 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