From fb4937a31185a967f874da1c1f679de7d0f9c16a Mon Sep 17 00:00:00 2001 From: Brad Spencer Date: Tue, 27 May 2014 23:58:28 +0200 Subject: [PATCH] select: with winsock, avoid passing unsupported arguments to select() "Any two of the parameters, readfds, writefds, or exceptfds, can be given as null. At least one must be non-null, and any non-null descriptor set must contain at least one handle to a socket." http://msdn.microsoft.com/en-ca/library/windows/desktop/ms740141(v=vs.85).aspx When using select(), cURL doesn't adhere to this (WinSock-specific) rule, and can ask to monitor empty fd_sets, which leads to select() returning WSAEINVAL (i.e. EINVAL) and connections failing in mysterious ways as a result (at least when using the curl_multi_socket_action() interface). Bug: http://curl.haxx.se/mail/lib-2014-05/0278.html --- lib/select.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 3 deletions(-) diff --git a/lib/select.c b/lib/select.c index db7fb6d43..da3082dda 100644 --- a/lib/select.c +++ b/lib/select.c @@ -5,7 +5,7 @@ * | (__| |_| | _ <| |___ * \___|\___/|_| \_\_____| * - * Copyright (C) 1998 - 2012, Daniel Stenberg, , et al. + * Copyright (C) 1998 - 2014, Daniel Stenberg, , et al. * * This software is licensed as described in the file COPYING, which * you should have received as part of this distribution. The terms @@ -288,7 +288,37 @@ int Curl_socket_check(curl_socket_t readfd0, /* two sockets to read from */ pending_tv.tv_sec = 0; pending_tv.tv_usec = 0; } - r = select((int)maxfd + 1, &fds_read, &fds_write, &fds_err, ptimeout); + + /* WinSock select() must not be called with an fd_set that contains zero + fd flags, or it will return WSAEINVAL. But, it also can't be called + with no fd_sets at all! From the documentation: + + Any two of the parameters, readfds, writefds, or exceptfds, can be + given as null. At least one must be non-null, and any non-null + descriptor set must contain at least one handle to a socket. + + We know that we have at least one bit set in at least two fd_sets in + this case, but we may have no bits set in either fds_read or fd_write, + so check for that and handle it. Luckily, with WinSock, we can _also_ + ask how many bits are set on an fd_set. + + It is unclear why WinSock doesn't just handle this for us instead of + calling this an error. + + Note also that WinSock ignores the first argument, so we don't worry + about the fact that maxfd is computed incorrectly with WinSock (since + curl_socket_t is unsigned in such cases and thus -1 is the largest + value). + */ + r = select((int)maxfd + 1, +#ifndef USE_WINSOCK + &fds_read, + &fds_write, +#else + fds_read.fd_count ? &fds_read : NULL, + fds_write.fd_count ? &fds_write : NULL, +#endif + &fds_err, ptimeout); if(r != -1) break; error = SOCKERRNO; @@ -446,6 +476,16 @@ int Curl_poll(struct pollfd ufds[], unsigned int nfds, int timeout_ms) } } +#ifdef USE_WINSOCK + /* WinSock select() can't handle zero events. See the comment about this in + Curl_check_socket(). */ + if(fds_read.fd_count == 0 && fds_write.fd_count == 0 + && fds_err.fd_count == 0) { + r = Curl_wait_ms(timeout_ms); + return r; + } +#endif + ptimeout = (timeout_ms < 0) ? NULL : &pending_tv; do { @@ -457,7 +497,19 @@ int Curl_poll(struct pollfd ufds[], unsigned int nfds, int timeout_ms) pending_tv.tv_sec = 0; pending_tv.tv_usec = 0; } - r = select((int)maxfd + 1, &fds_read, &fds_write, &fds_err, ptimeout); + r = select((int)maxfd + 1, +#ifndef USE_WINSOCK + &fds_read, &fds_write, &fds_err, +#else + /* WinSock select() can't handle fd_sets with zero bits set, so + don't give it such arguments. See the comment about this in + Curl_check_socket(). + */ + fds_read.fd_count ? &fds_read : NULL, + fds_write.fd_count ? &fds_write : NULL, + fds_err.fd_count ? &fds_err : NULL, +#endif + ptimeout); if(r != -1) break; error = SOCKERRNO;