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

[PATCH.HURD] Limit the minimum timeout delay to 1us for hurdselect.c to avoid problems



Hello,

I'm currently working with an update of hurdselect.c in eglibc, and
found a problem with non-blocking delays of value zero. This problem has
been seen before for vi for select and a workaround hack is currently in
the Debian version of eglibc-2.13. I have also seen this problem with my
new implementation of hurdselect, both for poll() and select(). The
updated code seems to be a little faster for the zero timeout case on my
kvm box, so the problems are revealed with the new version. This problem
is an old one, see Debian bug http://bugs.debian.org/79358. 

Attached is test code to check this problem with both poll() and
select() options. 
>From the Linux man-page for poll:
"Specifying a timeout of zero causes poll() to return immediately, even
if no file descriptors are ready." '

POSIX.1-2001 specifies ofr poll():
"If the value of timeout is 0, poll() shall return immediately,..."
not saying anything about file descriptors being ready or not.
and for select():
"To effect a poll, the timeout parameter should not be a null pointer,
and should point to a zero-valued timespec structure."

The attached patch is about the same as in Massage 53 of that bug,
limiting the smallest timeout to be 1us. The difference is that it does
not set the timeout to zero if it is given as NULL. This value seems to
work OK both for poll and select cases. Of course this value has been
found by trial and error, but when computers get faster and faster this
value would be an even better bet (unless other problems show up). The
patch is a diff of the patched Debian file.

/* Program: test_timeout.c */

#include <stdio.h>
#include <signal.h>
#include <string.h>
#include <errno.h>
#include <fcntl.h>
#include <time.h> /* timespec definition */
#include <poll.h>
#include <sys/select.h>
#include <sys/time.h> /* timespec definition */
#include <unistd.h>

int main(void)
{
  printf("\nPoll test\n");
  /* Data structure describing a polling request.  */
  //struct pollfd
  //{
  //  int fd;                     /* File descriptor to poll.  */
  //  short int events;           /* Types of events poller cares about.  */
  //  short int revents;          /* Types of events that actually occurred.  */
  //};

  int nfound = -1, nfds = -1;
  int timeout_msecs = 0;
  struct pollfd fds[1];

  nfound = -1;
  nfds = 1;
  fds[0].fd = 0;
  fds[0].events = POLLIN | POLLRDNORM | POLLOUT | POLLRDBAND;
  fds[0].revents = 0;
  timeout_msecs = 0;
  fprintf(stderr,"\nTest 1: timeout_msec=%d\n", timeout_msecs);
  printf("Input: nfound=%d, fds[0].{fd,events,revents}={%d,%d,%d}\n", nfound, fds[0].fd, fds[0].events, fds[0].revents);
  nfound = poll(fds, nfds, timeout_msecs);
  printf("Output: nfound=%d, fds[0].{fd,events,revents}={%d,%d,%d}\n", nfound, fds[0].fd, fds[0].events, fds[0].revents);

  nfound = -1;
  nfds = 1;
  fds[0].fd = 0;
  fds[0].events =  POLLIN | POLLRDNORM | POLLOUT | POLLRDBAND;
  fds[0].revents = 0;
  timeout_msecs = 1;
  fprintf(stderr,"\nTest 2: timeout_msec=%d\n", timeout_msecs);
  printf("Input: nfound=%d, fds[0].{fd,events,revents}={%d,%d,%d}\n", nfound, fds[0].fd, fds[0].events, fds[0].revents);
  nfound = poll(fds, nfds, timeout_msecs);
  printf("Output: nfound=%d, fds[0].{fd,events,revents}={%d,%d,%d}\n", nfound, fds[0].fd, fds[0].events, fds[0].revents);

  printf("\nSelect test\n");

// typedef struct
//  {
//  __fd_mask __fds_bits[__FD_SETSIZE / __NFDBITS];
// # define __FDS_BITS(set) ((set)->__fds_bits)
//  } fd_set;

// struct timeval {
//  __time_t tv_sec;            /* Seconds.  */
//  __suseconds_t tv_usec;      /* Microseconds.  */
// };

/* Code inspired by: http://bugs.debian.org/79358*/

  fd_set rfds;
  struct timeval timeout_usecs = {0,0};

  nfound = -1;
  nfds = 1;
  FD_ZERO(&rfds);
  FD_SET(0, &rfds);
  timeout_usecs.tv_sec = 0;
  timeout_usecs.tv_usec = 0;
  fprintf(stderr,"\nTest 1: timeout_usecs=(%ld,%ld)\n", timeout_usecs.tv_sec,timeout_usecs.tv_usec);
  printf ("Press Enter ");
  fflush(stdout);
  sleep(3);
  nfound = select(nfds, &rfds, NULL, NULL, &timeout_usecs);
  printf("Output: nfound=%d\n",nfound);

  nfound = -1;
  nfds = 1;
  FD_ZERO(&rfds);
  FD_SET(0, &rfds);
  timeout_usecs.tv_sec = 0;
  timeout_usecs.tv_usec = 1;
  fprintf(stderr,"\nTest 2: timeout_usecs=(%ld,%ld)\n", timeout_usecs.tv_sec,timeout_usecs.tv_usec);
  printf("Input: nfound=%d\n", nfound);
  printf ("Press Enter ");
  fflush(stdout);
  sleep(3);
  nfound = select(nfds, &rfds, NULL, NULL, &timeout_usecs);
  printf("Output: nfound=%d\n",nfound);


  return 0;
}
--- ../hurdselect_orig.c	2012-10-21 22:55:26.000000000 +0200
+++ ../hurdselect_orig_timeout.c	2012-11-01 12:58:00.000000000 +0100
@@ -84,9 +84,13 @@
 
       to = (timeout->tv_sec * 1000 +
             (timeout->tv_nsec + 999999) / 1000000);
-      if (strcmp(program_invocation_short_name, "vi") && strcmp(program_invocation_short_name, "vim") && strcmp(program_invocation_short_name, "vimdiff") && !to)
-	to = 1;
     }
+  /* XXX: A timeout of 0 returns immediately, even if no file
+     descriptors are ready. This is correct according to
+     POSIX.1-2001. As many programs rely on file descriptors being
+     ready for a timeout of zero use 1 msec as the minimum delay */
+  if (to == 0)
+    to = 1;
 
   if (sigmask && __sigprocmask (SIG_SETMASK, sigmask, &oset))
     return -1;

Reply to: