From d133cf839421462280ac0bfd9bd84c591f0e0249 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Fri, 27 Mar 2015 12:44:31 +0300 Subject: [PATCH] Support non-blocking connect on MS-Windows (Bug#20207) Based on ideas from Kim F. Storm , see http://lists.gnu.org/archive/html/emacs-devel/2006-12/msg00873.html. src/w32proc.c (reader_thread): If the FILE_CONNECT flag is set, call '_sys_wait_connect'. If it returns STATUS_CONNECT_FAILED, exit the thread with code 2. (sys_select): Support 'wfds' in addition to 'rfds'. If a descriptor in 'wfds' has its bit set, but the corresponding fd_info member doesn't have its FILE_CONNECT flag set, ignore the descriptor. Otherwise, acknowledge a successful non-blocking connect by resetting the FILE_CONNECT flag and setting cp->status to STATUS_READ_ACKNOWLEDGED. src/w32.h (STATUS_CONNECT_FAILED): New enumeration value. (struct _child_process): New member 'errcode'. (FILE_CONNECT): New flag. (_sys_wait_connect): Add prototype. src/w32.c (pfn_WSAEnumNetworkEvents): New function pointer. (init_winsock): Load WSAEnumNetworkEvents from winsock DLL. (set_errno): Map WSAEWOULDBLOCK and WSAENOTCONN. (sys_connect): Support non-blocking 'connect' calls by setting the FILE_CONNECT flag in the fd_info member and returning EINPROGRESS. (_sys_read_ahead): Add debug message if this function is called for a descriptor that waits for a non-blocking connect to complete. (_sys_wait_connect): New function. (sys_read): Support STATUS_CONNECT_FAILED. Return the error code recorded by _sys_wait_connect when the non-blocking connect failed. Don't call WSAGetLastError before a call to set_errno had a chance to use its value, since WSAGetLastError clears the last error. nt/inc/ms-w32.h (BROKEN_NON_BLOCKING_CONNECT): Don't define. --- nt/ChangeLog | 5 +++ nt/inc/ms-w32.h | 4 -- src/ChangeLog | 35 ++++++++++++++++ src/w32.c | 105 +++++++++++++++++++++++++++++++++++++++++++++--- src/w32.h | 9 ++++- src/w32proc.c | 77 ++++++++++++++++++++++++++++------- 6 files changed, 209 insertions(+), 26 deletions(-) diff --git a/nt/ChangeLog b/nt/ChangeLog index 9d954d56473..da796810947 100644 --- a/nt/ChangeLog +++ b/nt/ChangeLog @@ -1,3 +1,8 @@ +2015-03-27 Eli Zaretskii + + * inc/ms-w32.h (BROKEN_NON_BLOCKING_CONNECT): Don't define. + (Bug#20207) + 2015-03-09 Eli Zaretskii * INSTALL: Add some more installation instructions for mingw-get diff --git a/nt/inc/ms-w32.h b/nt/inc/ms-w32.h index c06ed588818..da772906ddc 100644 --- a/nt/inc/ms-w32.h +++ b/nt/inc/ms-w32.h @@ -58,10 +58,6 @@ along with GNU Emacs. If not, see . */ Look in for a timeval structure. */ #define HAVE_TIMEVAL 1 -/* But our select implementation doesn't allow us to make non-blocking - connects. So until that is fixed, this is necessary: */ -#define BROKEN_NON_BLOCKING_CONNECT 1 - /* And the select implementation does 1-byte read-ahead waiting for received packets, so datagrams are broken too. */ #define BROKEN_DATAGRAM_SOCKETS 1 diff --git a/src/ChangeLog b/src/ChangeLog index 632b798ce08..67e04f6f41c 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,38 @@ +2015-03-27 Eli Zaretskii + + Support non-blocking connect on MS-Windows. + Based on ideas from Kim F. Storm , see + http://lists.gnu.org/archive/html/emacs-devel/2006-12/msg00873.html. + + * w32proc.c (reader_thread): If the FILE_CONNECT flag is set, call + '_sys_wait_connect'. If it returns STATUS_CONNECT_FAILED, exit + the thread with code 2. + (sys_select): Support 'wfds' in addition to 'rfds'. If a + descriptor in 'wfds' has its bit set, but the corresponding + fd_info member doesn't have its FILE_CONNECT flag set, ignore the + descriptor. Otherwise, acknowledge a successful non-blocking + connect by resetting the FILE_CONNECT flag and setting cp->status + to STATUS_READ_ACKNOWLEDGED. (Bug#20207) + + * w32.h (STATUS_CONNECT_FAILED): New enumeration value. + (struct _child_process): New member 'errcode'. + (FILE_CONNECT): New flag. + (_sys_wait_connect): Add prototype. + + * w32.c (pfn_WSAEnumNetworkEvents): New function pointer. + (init_winsock): Load WSAEnumNetworkEvents from winsock DLL. + (set_errno): Map WSAEWOULDBLOCK and WSAENOTCONN. + (sys_connect): Support non-blocking 'connect' calls by setting the + FILE_CONNECT flag in the fd_info member and returning EINPROGRESS. + (_sys_read_ahead): Add debug message if this function is called + for a descriptor that waits for a non-blocking connect to complete. + (_sys_wait_connect): New function. + (sys_read): Support STATUS_CONNECT_FAILED. Return the error code + recorded by _sys_wait_connect when the non-blocking connect + failed. Don't call WSAGetLastError before a call to set_errno had + a chance to use its value, since WSAGetLastError clears the last + error. + 2015-03-25 Stefan Monnier * editfns.c (save_excursion_save): Don't save the mark. diff --git a/src/w32.c b/src/w32.c index 547db0f6dd9..1917fea343d 100644 --- a/src/w32.c +++ b/src/w32.c @@ -7038,6 +7038,9 @@ int (PASCAL *pfn_WSAStartup) (WORD wVersionRequired, LPWSADATA lpWSAData); void (PASCAL *pfn_WSASetLastError) (int iError); int (PASCAL *pfn_WSAGetLastError) (void); int (PASCAL *pfn_WSAEventSelect) (SOCKET s, HANDLE hEventObject, long lNetworkEvents); +int (PASCAL *pfn_WSAEnumNetworkEvents) (SOCKET s, HANDLE hEventObject, + WSANETWORKEVENTS *NetworkEvents); + HANDLE (PASCAL *pfn_WSACreateEvent) (void); int (PASCAL *pfn_WSACloseEvent) (HANDLE hEvent); int (PASCAL *pfn_socket) (int af, int type, int protocol); @@ -7123,6 +7126,7 @@ init_winsock (int load_now) LOAD_PROC (WSASetLastError); LOAD_PROC (WSAGetLastError); LOAD_PROC (WSAEventSelect); + LOAD_PROC (WSAEnumNetworkEvents); LOAD_PROC (WSACreateEvent); LOAD_PROC (WSACloseEvent); LOAD_PROC (socket); @@ -7206,6 +7210,8 @@ set_errno (void) case WSAEMFILE: errno = EMFILE; break; case WSAENAMETOOLONG: errno = ENAMETOOLONG; break; case WSAENOTEMPTY: errno = ENOTEMPTY; break; + case WSAEWOULDBLOCK: errno = EWOULDBLOCK; break; + case WSAENOTCONN: errno = ENOTCONN; break; default: errno = wsa_err; break; } } @@ -7473,8 +7479,18 @@ sys_connect (int s, const struct sockaddr * name, int namelen) { int rc = pfn_connect (SOCK_HANDLE (s), name, namelen); if (rc == SOCKET_ERROR) - set_errno (); - return rc; + { + set_errno (); + /* If this is a non-blocking 'connect', set the bit in flags + that will tell reader_thread to wait for connection + before trying to read. */ + if (errno == EWOULDBLOCK && (fd_info[s].flags & FILE_NDELAY) != 0) + { + errno = EINPROGRESS; /* that's what process.c expects */ + fd_info[s].flags |= FILE_CONNECT; + } + return rc; + } } errno = ENOTSOCK; return SOCKET_ERROR; @@ -7984,6 +8000,8 @@ _sys_read_ahead (int fd) emacs_abort (); } + if ((fd_info[fd].flags & FILE_CONNECT) != 0) + DebPrint (("_sys_read_ahead: read requested from fd %d, which waits for async connect!\n", fd)); cp->status = STATUS_READ_IN_PROGRESS; if (fd_info[fd].flags & FILE_PIPE) @@ -8105,6 +8123,60 @@ _sys_wait_accept (int fd) return cp->status; } +int +_sys_wait_connect (int fd) +{ + HANDLE hEv; + child_process * cp; + int rc; + + if (fd < 0 || fd >= MAXDESC) + return STATUS_READ_ERROR; + + cp = fd_info[fd].cp; + if (cp == NULL || cp->fd != fd || cp->status != STATUS_READ_READY) + return STATUS_READ_ERROR; + + cp->status = STATUS_READ_FAILED; + + hEv = pfn_WSACreateEvent (); + rc = pfn_WSAEventSelect (SOCK_HANDLE (fd), hEv, FD_CONNECT); + if (rc != SOCKET_ERROR) + { + do { + rc = WaitForSingleObject (hEv, 500); + Sleep (5); + } while (rc == WAIT_TIMEOUT + && cp->status != STATUS_READ_ERROR + && cp->char_avail); + if (rc == WAIT_OBJECT_0) + { + /* We've got an event, but it could be a successful + connection, or it could be a failure. Find out + which one is it. */ + WSANETWORKEVENTS events; + + pfn_WSAEnumNetworkEvents (SOCK_HANDLE (fd), hEv, &events); + if ((events.lNetworkEvents & FD_CONNECT) != 0 + && events.iErrorCode[FD_CONNECT_BIT]) + { + cp->status = STATUS_CONNECT_FAILED; + cp->errcode = events.iErrorCode[FD_CONNECT_BIT]; + } + else + { + cp->status = STATUS_READ_SUCCEEDED; + cp->errcode = 0; + } + } + pfn_WSAEventSelect (SOCK_HANDLE (fd), NULL, 0); + } + else + pfn_WSACloseEvent (hEv); + + return cp->status; +} + int sys_read (int fd, char * buffer, unsigned int count) { @@ -8174,6 +8246,7 @@ sys_read (int fd, char * buffer, unsigned int count) ResetEvent (cp->char_avail); case STATUS_READ_ACKNOWLEDGED: + case STATUS_CONNECT_FAILED: break; default: @@ -8239,7 +8312,29 @@ sys_read (int fd, char * buffer, unsigned int count) { if (winsock_lib == NULL) emacs_abort (); - /* do the equivalent of a non-blocking read */ + /* When a non-blocking 'connect' call fails, + wait_reading_process_output detects this by calling + 'getpeername', and then attempts to obtain the connection + error code by trying to read 1 byte from the socket. If + we try to serve that read by calling 'recv' below, the + error we get is a generic WSAENOTCONN, not the actual + connection error. So instead, we use the actual error + code stashed by '_sys_wait_connect' in cp->errcode. + Alternatively, we could have used 'getsockopt', like on + GNU/Linux, but: (a) I have no idea whether the winsock + version could hang, as it does "on some systems" (see the + comment in process.c); and (b) 'getsockopt' on Windows is + documented to clear the socket error for the entire + process, which I'm not sure is TRT; FIXME. */ + if (current_status == STATUS_CONNECT_FAILED + && (fd_info[fd].flags & FILE_CONNECT) != 0 + && cp->errcode != 0) + { + pfn_WSASetLastError (cp->errcode); + set_errno (); + return -1; + } + /* Do the equivalent of a non-blocking read. */ pfn_ioctlsocket (SOCK_HANDLE (fd), FIONREAD, &waiting); if (waiting == 0 && nchars == 0) { @@ -8253,9 +8348,9 @@ sys_read (int fd, char * buffer, unsigned int count) int res = pfn_recv (SOCK_HANDLE (fd), buffer, count, 0); if (res == SOCKET_ERROR) { - DebPrint (("sys_read.recv failed with error %d on socket %ld\n", - pfn_WSAGetLastError (), SOCK_HANDLE (fd))); set_errno (); + DebPrint (("sys_read.recv failed with error %d on socket %ld\n", + errno, SOCK_HANDLE (fd))); return -1; } nchars += res; diff --git a/src/w32.h b/src/w32.h index 835557d5ec7..9b3521d077f 100644 --- a/src/w32.h +++ b/src/w32.h @@ -61,7 +61,8 @@ enum { STATUS_READ_IN_PROGRESS, STATUS_READ_FAILED, STATUS_READ_SUCCEEDED, - STATUS_READ_ACKNOWLEDGED + STATUS_READ_ACKNOWLEDGED, + STATUS_CONNECT_FAILED }; /* This structure is used for both pipes and sockets; for @@ -96,6 +97,8 @@ typedef struct _child_process /* Status of subprocess/connection and of reading its output. For values, see the enumeration above. */ volatile int status; + /* Used to store errno value of failed async 'connect' calls. */ + volatile int errcode; /* Holds a single character read by _sys_read_ahead, when a subprocess has some output ready. */ char chr; @@ -122,7 +125,8 @@ extern filedesc fd_info [ MAXDESC ]; /* fd_info flag definitions */ #define FILE_READ 0x0001 #define FILE_WRITE 0x0002 -#define FILE_LISTEN 0x0004 +#define FILE_LISTEN 0x0004 +#define FILE_CONNECT 0x0008 #define FILE_BINARY 0x0010 #define FILE_LAST_CR 0x0020 #define FILE_AT_EOF 0x0040 @@ -171,6 +175,7 @@ extern void init_timers (void); extern int _sys_read_ahead (int fd); extern int _sys_wait_accept (int fd); +extern int _sys_wait_connect (int fd); extern HMODULE w32_delayed_load (Lisp_Object); diff --git a/src/w32proc.c b/src/w32proc.c index 74731db2426..2d10534aa47 100644 --- a/src/w32proc.c +++ b/src/w32proc.c @@ -1011,7 +1011,9 @@ reader_thread (void *arg) { int rc; - if (cp->fd >= 0 && fd_info[cp->fd].flags & FILE_LISTEN) + if (cp->fd >= 0 && (fd_info[cp->fd].flags & FILE_CONNECT) != 0) + rc = _sys_wait_connect (cp->fd); + else if (cp->fd >= 0 && (fd_info[cp->fd].flags & FILE_LISTEN) != 0) rc = _sys_wait_accept (cp->fd); else rc = _sys_read_ahead (cp->fd); @@ -1031,8 +1033,8 @@ reader_thread (void *arg) return 1; } - if (rc == STATUS_READ_ERROR) - return 1; + if (rc == STATUS_READ_ERROR || rc == STATUS_CONNECT_FAILED) + return 2; /* If the read died, the child has died so let the thread die */ if (rc == STATUS_READ_FAILED) @@ -1929,7 +1931,7 @@ int sys_select (int nfds, SELECT_TYPE *rfds, SELECT_TYPE *wfds, SELECT_TYPE *efds, struct timespec *timeout, void *ignored) { - SELECT_TYPE orfds; + SELECT_TYPE orfds, owfds; DWORD timeout_ms, start_time; int i, nh, nc, nr; DWORD active; @@ -1947,15 +1949,27 @@ sys_select (int nfds, SELECT_TYPE *rfds, SELECT_TYPE *wfds, SELECT_TYPE *efds, return 0; } - /* Otherwise, we only handle rfds, so fail otherwise. */ - if (rfds == NULL || wfds != NULL || efds != NULL) + /* Otherwise, we only handle rfds and wfds, so fail otherwise. */ + if ((rfds == NULL && wfds == NULL) || efds != NULL) { errno = EINVAL; return -1; } - orfds = *rfds; - FD_ZERO (rfds); + if (rfds) + { + orfds = *rfds; + FD_ZERO (rfds); + } + else + FD_ZERO (&orfds); + if (wfds) + { + owfds = *wfds; + FD_ZERO (wfds); + } + else + FD_ZERO (&owfds); nr = 0; /* If interrupt_handle is available and valid, always wait on it, to @@ -1970,7 +1984,7 @@ sys_select (int nfds, SELECT_TYPE *rfds, SELECT_TYPE *wfds, SELECT_TYPE *efds, /* Build a list of pipe handles to wait on. */ for (i = 0; i < nfds; i++) - if (FD_ISSET (i, &orfds)) + if (FD_ISSET (i, &orfds) || FD_ISSET (i, &owfds)) { if (i == 0) { @@ -1984,7 +1998,7 @@ sys_select (int nfds, SELECT_TYPE *rfds, SELECT_TYPE *wfds, SELECT_TYPE *efds, /* Check for any emacs-generated input in the queue since it won't be detected in the wait */ - if (detect_input_pending ()) + if (rfds && detect_input_pending ()) { FD_SET (i, rfds); return 1; @@ -1999,6 +2013,13 @@ sys_select (int nfds, SELECT_TYPE *rfds, SELECT_TYPE *wfds, SELECT_TYPE *efds, { /* Child process and socket/comm port input. */ cp = fd_info[i].cp; + if (FD_ISSET (i, &owfds) + && cp + && (fd_info[i].flags && FILE_CONNECT) == 0) + { + DebPrint (("sys_select: fd %d is in wfds, but FILE_CONNECT is reset!\n", i)); + cp = NULL; + } if (cp) { int current_status = cp->status; @@ -2007,6 +2028,8 @@ sys_select (int nfds, SELECT_TYPE *rfds, SELECT_TYPE *wfds, SELECT_TYPE *efds, { /* Tell reader thread which file handle to use. */ cp->fd = i; + /* Zero out the error code. */ + cp->errcode = 0; /* Wake up the reader thread for this process */ cp->status = STATUS_READ_READY; if (!SetEvent (cp->char_consumed)) @@ -2197,7 +2220,7 @@ count_children: if (cp->fd >= 0 && (fd_info[cp->fd].flags & FILE_AT_EOF) == 0) fd_info[cp->fd].flags |= FILE_SEND_SIGCHLD; - /* SIG_DFL for SIGCHLD is ignore */ + /* SIG_DFL for SIGCHLD is ignored */ else if (sig_handlers[SIGCHLD] != SIG_DFL && sig_handlers[SIGCHLD] != SIG_IGN) { @@ -2214,7 +2237,7 @@ count_children: errno = EINTR; return -1; } - else if (fdindex[active] == 0) + else if (rfds && fdindex[active] == 0) { /* Keyboard input available */ FD_SET (0, rfds); @@ -2222,9 +2245,33 @@ count_children: } else { - /* must be a socket or pipe - read ahead should have - completed, either succeeding or failing. */ - FD_SET (fdindex[active], rfds); + /* Must be a socket or pipe - read ahead should have + completed, either succeeding or failing. If this handle + was waiting for an async 'connect', reset the connect + flag, so it could read from now on. */ + if (wfds && (fd_info[fdindex[active]].flags & FILE_CONNECT) != 0) + { + cp = fd_info[fdindex[active]].cp; + if (cp) + { + /* Don't reset the FILE_CONNECT bit and don't + acknowledge the read if the status is + STATUS_CONNECT_FAILED or some other + failure. That's because the thread exits in those + cases, so it doesn't need the ACK, and we want to + keep the FILE_CONNECT bit as evidence that the + connect failed, to be checked in sys_read. */ + if (cp->status == STATUS_READ_SUCCEEDED) + { + fd_info[cp->fd].flags &= ~FILE_CONNECT; + cp->status = STATUS_READ_ACKNOWLEDGED; + } + ResetEvent (cp->char_avail); + } + FD_SET (fdindex[active], wfds); + } + else if (rfds) + FD_SET (fdindex[active], rfds); nr++; } -- 2.39.5