Re: gnulib poll() returns POLLERR on Win32 platform, breaking libvirt

[Paolo Bonzini]

On 18/04/2017 17:07, Daniel P. Berrange wrote:
> We're seeing a failure in libvirt on Win32 platforms whereby poll() often
> returns POLLERR. I traced this down to the rpl_recv() function calling
> FD_TO_SOCKET() and getting INVALID_SOCKET back. This is propagated back
> to windows_compute_revents_socket() which sets POLLERR, because
> WSAGetLastError() returns 0.
> 
> The windows_compute_revents_socket() is indeed passing a bad file descriptor
> to recv, because it is actually passing in a SOCKET handle.
> 
> After bisecting gnulib, I found the root cause of this problem is this commit:
> 
>   commit 17f1e64f00011fb745019119e21b26e4aba65e4b
>   Author: Paul Eggert <address@hidden>
>   Date:   Tue Feb 24 16:16:19 2015 -0800
> 
>     poll: port to MSVC v18 on MS-Windows 8.1
>     
>     Problem reported by Gisle Vanem in:
>     http://lists.gnu.org/archive/html/bug-gnulib/2015-02/msg00139.html
>     * lib/poll.c: Always include <sys/select.h> and <sys/socket.h>.
>     * modules/poll (Depends-on) [!HAVE_POLL || REPLACE_POLL]:
>     Add sys_socket.
> 
> 
> by unconditionally pulling in sys/select.c & sys/socket.c, we are causing
> poll() to call gnulib's replacement rpl_recv() wrapper, rather than the
> original bare Winsock recv() method.
> 
> Reverting that change fixes libvirt, but obviously that's not a valid
> approach, since it'll re-break  MSVC v18 on Win8.1
> 
> 
> So I tried the following change to make it pass in an FD into the rpl_recv
> method instead of a HANDLE
> 
> @@ -571,7 +568,7 @@
>            if (FD_ISSET ((SOCKET) h, &xfds))
>              ev.lNetworkEvents |= FD_OOB;
>  
> -          happened = windows_compute_revents_socket (pfd[i].fd, 
> pfd[i].events,
> +          happened = windows_compute_revents_socket ((SOCKET) h, 
> pfd[i].events,
>                                                       ev.lNetworkEvents);
>          }
>        else

You mean you applied the reverse of this patch, of course?  For a
complete patch you'd also change windows_compute_revents_socket's first
argument to "int fd", and change it to use errno instead of WSAGetLastError.

But perhaps it's simpler to add "#undef recv" and "#undef select" near
the top of the file, in addition to this change.

Thanks,

Paolo