bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Daniel P. Berrange
Subject: Re: gnulib poll() returns POLLERR on Win32 platform, breaking libvirt
Date: Wed, 19 Apr 2017 16:50:11 +0100
User-agent: Mutt/1.7.1 (2016-10-04)

On Wed, Apr 19, 2017 at 05:36:54PM +0200, Paolo Bonzini wrote:
> 
> 
> 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

Sigh, yes.

> 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.

Yep, I changed the arg, but didn't try the WSAGetLastError change.

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

If we add the undef's then we don't need to change anything related to
windows_compute_revents_socket() as it'd be calling the native recv()
which expects the SOCKET rather than fd.

If we undef those functions, would that not re-introduce the bug mentioned
in http://lists.gnu.org/archive/html/bug-gnulib/2015-02/msg00139.html  ?

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



reply via email to

[Prev in Thread] Current Thread [Next in Thread]