changeset 11852:c9032834d889

Fix polling for writeability of a screen buffer. * lib/poll.c: Distinguish input and screen buffers for the Win32 implementation. * lib/select.c: Likewise.
author Paolo Bonzini <bonzini@gnu.org>
date Tue, 04 Aug 2009 18:06:07 +0200
parents c9727d4e7e5a
children 97a383c7cec4
files ChangeLog lib/poll.c lib/select.c
diffstat 3 files changed, 123 insertions(+), 42 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2009-08-20  Paolo Bonzini  <bonzini@gnu.org>
+
+	Fix polling for writeability of a screen buffer.
+	* lib/poll.c: Distinguish input and screen buffers for the
+	Win32 implementation.
+	* lib/select.c: Likewise.
+
 2009-08-19  Eric Blake  <ebb9@byu.net>
 
 	popen-safer: prevent popen from clobbering std descriptors
--- a/lib/poll.c
+++ b/lib/poll.c
@@ -67,6 +67,23 @@
 
 #ifdef WIN32_NATIVE
 
+#define IsConsoleHandle(h) (((long) (h) & 3) == 3)
+
+static BOOL
+IsSocketHandle(HANDLE h)
+{
+  WSANETWORKEVENTS ev;
+
+  if (IsConsoleHandle (h))
+    return FALSE;
+
+  /* Under Wine, it seems that getsockopt returns 0 for pipes too.
+     WSAEnumNetworkEvents instead distinguishes the two correctly.  */
+  ev.lNetworkEvents = 0xDEADBEEF;
+  WSAEnumNetworkEvents ((SOCKET) h, NULL, &ev);
+  return ev.lNetworkEvents != 0xDEADBEEF;
+}
+
 /* Declare data structures for ntdll functions.  */
 typedef struct _FILE_PIPE_LOCAL_INFORMATION {
   ULONG NamedPipeType;
@@ -101,10 +118,11 @@
 #  define PIPE_BUF	512
 # endif
 
-/* Compute revents values for file handle H.  */
+/* Compute revents values for file handle H.  If some events cannot happen
+   for the handle, eliminate them from *P_SOUGHT.  */
 
 static int
-win32_compute_revents (HANDLE h, int sought)
+win32_compute_revents (HANDLE h, int *p_sought)
 {
   int i, ret, happened;
   INPUT_RECORD *irbuffer;
@@ -130,7 +148,7 @@
       if (PeekNamedPipe (h, NULL, 0, NULL, &avail, NULL) != 0)
 	{
 	  if (avail)
-	    happened |= sought & (POLLIN | POLLRDNORM);
+	    happened |= *p_sought & (POLLIN | POLLRDNORM);
 	}
 
       else
@@ -151,18 +169,25 @@
 	      || fpli.WriteQuotaAvailable >= PIPE_BUF
 	      || (fpli.OutboundQuota < PIPE_BUF &&
 	          fpli.WriteQuotaAvailable == fpli.OutboundQuota))
-	    happened |= sought & (POLLOUT | POLLWRNORM | POLLWRBAND);
+	    happened |= *p_sought & (POLLOUT | POLLWRNORM | POLLWRBAND);
 	}
       return happened;
 
     case FILE_TYPE_CHAR:
       ret = WaitForSingleObject (h, 0);
-      if (ret == WAIT_OBJECT_0)
-        {
-	  nbuffer = avail = 0;
-	  bRet = GetNumberOfConsoleInputEvents (h, &nbuffer);
-	  if (!bRet || nbuffer == 0)
+      if (!IsConsoleHandle (h))
+	return ret == WAIT_OBJECT_0 ? *p_sought & ~(POLLPRI | POLLRDBAND) : 0;
+
+      nbuffer = avail = 0;
+      bRet = GetNumberOfConsoleInputEvents (h, &nbuffer);
+      if (bRet)
+	{
+	  /* Input buffer.  */
+	  *p_sought &= POLLIN | POLLRDNORM;
+	  if (nbuffer == 0)
 	    return POLLHUP;
+	  if (!*p_sought)
+	    return 0;
 
 	  irbuffer = (INPUT_RECORD *) alloca (nbuffer * sizeof (INPUT_RECORD));
 	  bRet = PeekConsoleInput (h, irbuffer, nbuffer, &avail);
@@ -171,19 +196,23 @@
 
 	  for (i = 0; i < avail; i++)
 	    if (irbuffer[i].EventType == KEY_EVENT)
-	      return sought & ~(POLLPRI | POLLRDBAND);
+	      return *p_sought;
+	  return 0;
 	}
-      break;
+      else
+	{
+	  /* Screen buffer.  */
+	  *p_sought &= POLLOUT | POLLWRNORM | POLLWRBAND;
+	  return *p_sought;
+	}
 
     default:
       ret = WaitForSingleObject (h, 0);
       if (ret == WAIT_OBJECT_0)
-        return sought & ~(POLLPRI | POLLRDBAND);
+        return *p_sought & ~(POLLPRI | POLLRDBAND);
 
-      break;
+      return *p_sought & (POLLOUT | POLLWRNORM | POLLWRBAND);
     }
-
-  return sought & (POLLOUT | POLLWRNORM | POLLWRBAND);
 }
 
 /* Convert fd_sets returned by select into revents values.  */
@@ -430,36 +459,32 @@
   /* Classify socket handles and create fd sets. */
   for (i = 0; i < nfd; i++)
     {
+      int sought = pfd[i].events;
       pfd[i].revents = 0;
       if (pfd[i].fd < 0)
         continue;
-      if (!(pfd[i].events & (POLLIN | POLLRDNORM |
-                             POLLOUT | POLLWRNORM | POLLWRBAND)))
+      if (!(sought & (POLLIN | POLLRDNORM | POLLOUT | POLLWRNORM | POLLWRBAND
+		      | POLLPRI | POLLRDBAND)))
 	continue;
 
       h = (HANDLE) _get_osfhandle (pfd[i].fd);
       assert (h != NULL);
-
-      /* Under Wine, it seems that getsockopt returns 0 for pipes too.
-	 WSAEnumNetworkEvents instead distinguishes the two correctly.  */
-      ev.lNetworkEvents = 0xDEADBEEF;
-      WSAEnumNetworkEvents ((SOCKET) h, NULL, &ev);
-      if (ev.lNetworkEvents != 0xDEADBEEF)
+      if (IsSocketHandle (h))
         {
           int requested = FD_CLOSE;
 
           /* see above; socket handles are mapped onto select.  */
-          if (pfd[i].events & (POLLIN | POLLRDNORM))
+          if (sought & (POLLIN | POLLRDNORM))
 	    {
               requested |= FD_READ | FD_ACCEPT;
 	      FD_SET ((SOCKET) h, &rfds);
 	    }
-          if (pfd[i].events & (POLLOUT | POLLWRNORM | POLLWRBAND))
+          if (sought & (POLLOUT | POLLWRNORM | POLLWRBAND))
 	    {
               requested |= FD_WRITE | FD_CONNECT;
 	      FD_SET ((SOCKET) h, &wfds);
 	    }
-          if (pfd[i].events & (POLLPRI | POLLRDBAND))
+          if (sought & (POLLPRI | POLLRDBAND))
 	    {
               requested |= FD_OOB;
 	      FD_SET ((SOCKET) h, &xfds);
@@ -470,10 +495,13 @@
         }
       else
         {
-          handle_array[nhandles++] = h;
-
-	  /* Poll now.  If we get an event, do not poll again.  */
-          pfd[i].revents = win32_compute_revents (h, pfd[i].events);
+	  /* Poll now.  If we get an event, do not poll again.  Also,
+	     screen buffer handles are waitable, and they'll block until
+	     a character is available.  win32_compute_revents eliminates
+	     bits for the "wrong" direction. */
+          pfd[i].revents = win32_compute_revents (h, &sought);
+	  if (sought)
+	    handle_array[nhandles++] = h;
           if (pfd[i].revents)
 	    wait_timeout = 0;
         }
@@ -553,8 +581,9 @@
       else
         {
           /* Not a socket.  */
+          int sought = pfd[i].events;
+          happened = win32_compute_revents (h, &sought);
           nhandles++;
-          happened = win32_compute_revents (h, pfd[i].events);
         }
 
        if ((pfd[i].revents |= happened) != 0)
--- a/lib/select.c
+++ b/lib/select.c
@@ -21,6 +21,7 @@
 
 #include <config.h>
 #include <alloca.h>
+#include <assert.h>
 
 #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
 /* Native Win32.  */
@@ -76,6 +77,23 @@
 #define PIPE_BUF	512
 #endif
 
+#define IsConsoleHandle(h) (((long) (h) & 3) == 3)
+
+static BOOL
+IsSocketHandle(HANDLE h)
+{
+  WSANETWORKEVENTS ev;
+
+  if (IsConsoleHandle (h))
+    return FALSE;
+
+  /* Under Wine, it seems that getsockopt returns 0 for pipes too.
+     WSAEnumNetworkEvents instead distinguishes the two correctly.  */
+  ev.lNetworkEvents = 0xDEADBEEF;
+  WSAEnumNetworkEvents ((SOCKET) h, NULL, &ev);
+  return ev.lNetworkEvents != 0xDEADBEEF;
+}
+
 /* Compute output fd_sets for libc descriptor FD (whose Win32 handle is H).  */
 
 static int
@@ -138,19 +156,37 @@
       break;
 
     case FILE_TYPE_CHAR:
+      write = TRUE;
+      if (!(rbits->in[fd / CHAR_BIT] & (1 << (fd & (CHAR_BIT - 1)))))
+	break;
+
       ret = WaitForSingleObject (h, 0);
-      write = TRUE;
       if (ret == WAIT_OBJECT_0)
         {
+	  if (!IsConsoleHandle (h))
+	    {
+	      read = TRUE;
+	      break;
+	    }
+
 	  nbuffer = avail = 0;
 	  bRet = GetNumberOfConsoleInputEvents (h, &nbuffer);
-	  if (!bRet || nbuffer == 0)
-	    except = TRUE;
+
+	  /* Screen buffers handles are filtered earlier.  */
+	  assert (bRet);
+	  if (nbuffer == 0)
+	    {
+	      except = TRUE;
+	      break;
+	    }
 
 	  irbuffer = (INPUT_RECORD *) alloca (nbuffer * sizeof (INPUT_RECORD));
 	  bRet = PeekConsoleInput (h, irbuffer, nbuffer, &avail);
 	  if (!bRet || avail == 0)
-	    except = TRUE;
+	    {
+	      except = TRUE;
+	      break;
+	    }
 
 	  for (i = 0; i < avail; i++)
 	    if (irbuffer[i].EventType == KEY_EVENT)
@@ -199,7 +235,7 @@
   fd_set handle_rfds, handle_wfds, handle_xfds;
   struct bitset rbits, wbits, xbits;
   unsigned char anyfds_in[FD_SETSIZE / CHAR_BIT];
-  DWORD ret, wait_timeout, nhandles, nsock;
+  DWORD ret, wait_timeout, nhandles, nsock, nbuffer;
   MSG msg;
   int i, fd, rc;
 
@@ -227,7 +263,10 @@
   nhandles = 1;
   nsock = 0;
 
-  /* Copy descriptors to bitsets.  */
+  /* Copy descriptors to bitsets.  At the same time, eliminate
+     bits in the "wrong" direction for console input buffers
+     and screen buffers, because screen buffers are waitable
+     and they will block until a character is available.  */
   memset (&rbits, 0, sizeof (rbits));
   memset (&wbits, 0, sizeof (wbits));
   memset (&xbits, 0, sizeof (xbits));
@@ -236,6 +275,11 @@
     for (i = 0; i < rfds->fd_count; i++)
       {
         fd = rfds->fd_array[i];
+	h = (HANDLE) _get_osfhandle (fd);
+	if (IsConsoleHandle (h)
+	    && !GetNumberOfConsoleInputEvents (h, &nbuffer))
+	  continue;
+
         rbits.in[fd / CHAR_BIT] |= 1 << (fd & (CHAR_BIT - 1));
         anyfds_in[fd / CHAR_BIT] |= 1 << (fd & (CHAR_BIT - 1));
       }
@@ -246,6 +290,11 @@
     for (i = 0; i < wfds->fd_count; i++)
       {
         fd = wfds->fd_array[i];
+	h = (HANDLE) _get_osfhandle (fd);
+	if (IsConsoleHandle (h)
+	    && GetNumberOfConsoleInputEvents (h, &nbuffer))
+	  continue;
+
         wbits.in[fd / CHAR_BIT] |= 1 << (fd & (CHAR_BIT - 1));
         anyfds_in[fd / CHAR_BIT] |= 1 << (fd & (CHAR_BIT - 1));
       }
@@ -285,11 +334,7 @@
 	  return -1;
         }
 
-      /* Under Wine, it seems that getsockopt returns 0 for pipes too.
-	 WSAEnumNetworkEvents instead distinguishes the two correctly.  */
-      ev.lNetworkEvents = 0xDEADBEEF;
-      WSAEnumNetworkEvents ((SOCKET) h, NULL, &ev);
-      if (ev.lNetworkEvents != 0xDEADBEEF)
+      if (IsSocketHandle (h))
         {
           int requested = FD_CLOSE;