Re: [PATCH/RFC 01/11] mingw: add network-wrappers for daemon

[Erik Faye-Lund <kusmabite@googlemail.com>]

(CC'ing Hannes, as he seems to be the original author of our poll()-emulation)

On Thu, Nov 26, 2009 at 12:03 PM, Martin Storsjö <martin@martin.st> wrote:
> On Thu, 26 Nov 2009, Erik Faye-Lund wrote:
>
>> Yeah, I saw your patches, and realized that I needed to rebase my work
>> at some point, but none of the repos I usually pull from seems to
>> contain the patches yet. Rebasing will be a requirement before this
>> can be applied for sure.
>
> Ok, great! I tried applying it on the latest master, and it wasn't too
> much of work.

I've finally gotten around to doing the same now, thanks for the heads up :)

> This causes problems with the mingw poll() replacement, which has a
> special case for polling one single fd - otherwise it tries to use some
> emulation that currently only works for pipes. I didn't try to make any
> proper fix for this though. I tested git-daemon by hacking it to listen on
> only one of the sockets, and that worked well for both v4 and v6.
>
>
> So, in addition to the getaddrinfo patch I sent, the mingw poll()
> replacement needs some updates to handle polling multiple sockets. Except
> from that, things seem to work, at a quick glance.
>

Thanks a lot for helping out! I'm seeing the same issue here when
running on Vista:
$ ./git-daemon.exe --base-path=/c/Users/Erik/src/ --export-all
error: PeekNamedPipe failed, GetLastError: 1
[3656] Poll failed, resuming: Invalid argument
error: PeekNamedPipe failed, GetLastError: 1
[3656] Poll failed, resuming: Invalid argument
...

If I hack the poll()-emulation to never enter the PeekNamedPipe
code-path seems to make it work with IPv6 here, but this is obviously
not the correct solution as it breaks pipe-polling:
--->8---
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -360,7 +360,7 @@ int poll(struct pollfd *ufds, unsigned int nfds, int timeout
         * input is available and let the actual wait happen when the
         * caller invokes read().
         */
-       if (nfds == 1) {
+       if (1) {
                if (!(ufds[0].events & POLLIN))
                        return errno = EINVAL, error("POLLIN not set");
                ufds[0].revents = POLLIN;
--->8---

I'm not very familiar with poll(), but if I understand the man-pages
correctly it's waiting for events on file descriptors, and is in our
case used to check for incoming connections, right? If so, I see three
possible ways forward: (1) extending our poll()-emulation to handle
multiple sockets, (2) change daemon.c to check one socket at the time,
and (3) using select() instead of poll().

(1) seems like the "correct" but tricky thing to do, (2) like the
"easy" but nasty thing to do. However, (3) strikes me as the least
dangerous thing to do ;)

For (1), there's also a WSAPoll() function in Windows, but I'm not
sure how to figure out if an fd is a socket or a pipe. There's also
WaitForMultipleObjects.

Here's a quick stab at (3). It seems to work fine under Windows. Not
tested on other platforms, though.

--->8---
--- a/daemon.c
+++ b/daemon.c
@@ -812,26 +812,22 @@ static int socksetup(char *listen_addr, int listen_port, i
nt **socklist_p)

 static int service_loop(int socknum, int *socklist)
 {
-       struct pollfd *pfd;
-       int i;
-
-       pfd = xcalloc(socknum, sizeof(struct pollfd));
-
-       for (i = 0; i < socknum; i++) {
-               pfd[i].fd = socklist[i];
-               pfd[i].events = POLLIN;
-       }
-
        signal(SIGCHLD, child_handler);

        for (;;) {
                int i;
+               fd_set fds;
+               struct timeval timeout = { 0, 0 };

                check_dead_children();

-               if (poll(pfd, socknum, -1) < 0) {
+               FD_ZERO(&fds);
+               for (i = 0; i < socknum; i++)
+                       FD_SET(socklist[i], &fds);
+
+               if (select(0, &fds, NULL, NULL, &timeout) > 0) {
                        if (errno != EINTR) {
-                               logerror("Poll failed, resuming: %s",
+                               logerror("select() failed, resuming: %s",
                                      strerror(errno));
                                sleep(1);
                        }
@@ -839,10 +835,10 @@ static int service_loop(int socknum, int *socklist)
                }

                for (i = 0; i < socknum; i++) {
-                       if (pfd[i].revents & POLLIN) {
+                       if (FD_ISSET(socklist[i], &fds)) {
                                struct sockaddr_storage ss;
                                socklen_t sslen = sizeof(ss);
-                               int incoming = accept(pfd[i].fd,
(struct sockaddr *)&ss, &sslen);
+                               int incoming = accept(socklist[i],
(struct sockaddr *)&ss, &sslen);
                                if (incoming < 0) {
                                        switch (errno) {
                                        case EAGAIN:
@@ -854,6 +850,7 @@ static int service_loop(int socknum, int *socklist)
                                        }
                                }
                                handle(incoming, (struct sockaddr *)&ss, sslen);

+                               break;
                        }
                }
        }
--->8---

-- 
Erik "kusma" Faye-Lund