poll() bugs and patch

poll() bugs and patch

[Boris Schaeling]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

There are several bugs in poll():
- poll() must return 0 if no descriptor is ready and the timer expires. It
works correctly if all descriptors are non-negative. Otherwise poll()
returns the count of negative descriptors which is wrong.
- The implementation of poll() must call cygwin_select() even if all
descriptors are invalid. Currently poll() returns immediately - and ignores
any timer value. The return value is wrong again because of the bug above.
- If the timer expires revents must be 0 and must not be set to anything
else like POLLNVAL, POLLHUP or whatever.
- If cygwin_select() returns -1 revents must be 0 and must not be set to
POLLERR. Eg. a signal interrupting poll() doesn't mean an error has occured
for a TCP connection.
- If an error is pending revents must be set to POLLERR.
- The implementation of poll() has a local variable called open_fds that
isn't used. I can't see why this variable exists.

I've attached three files:
- newpoll.cc has a new implementation of poll() which fixes the bugs.
- patch.cc is the patch.
- polltest.c is a testcase.

I rebuilt cygwin1.dll with my poll() function so there should be no problems
when compiling.

Boris

[-- Attachment #2: newpoll.cc --]
[-- Type: application/octet-stream, Size: 2472 bytes --]

/* poll.cc. Implements poll(2) via usage of select(2) call.

   Copyright 2000, 2001 Red Hat, Inc.

   This file is part of Cygwin.

   This software is a copyrighted work licensed under the terms of the
   Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
   details. */

#include "winsup.h"
#include <sys/time.h>
#include <sys/poll.h>
#include <errno.h>
#include <stdlib.h>
#include "security.h"
#include "fhandler.h"
#include "path.h"
#include "dtable.h"
#include "cygerrno.h"
#include "cygheap.h"
#include "sigproc.h"

extern "C"
int
poll (struct pollfd *fds, unsigned int nfds, int timeout)
{
  int max_fd = 0;
  fd_set *read_fds, *write_fds, *except_fds;
  struct timeval tv = { timeout / 1000, (timeout % 1000) * 1000 };
  sigframe thisframe (mainthread);

  for (unsigned int i = 0; i < nfds; ++i)
    if (fds[i].fd > max_fd)
      max_fd = fds[i].fd;

  size_t fds_size = howmany(max_fd + 1, NFDBITS) * sizeof (fd_mask);

  read_fds = (fd_set *) alloca (fds_size);
  write_fds = (fd_set *) alloca (fds_size);
  except_fds = (fd_set *) alloca (fds_size);

  if (!read_fds || !write_fds || !except_fds)
    {
      set_errno (ENOMEM);
      return -1;
    }

  memset (read_fds, 0, fds_size);
  memset (write_fds, 0, fds_size);
  memset (except_fds, 0, fds_size);

  for (unsigned int i = 0; i < nfds; ++i) 
  { 
    fds[i].revents = 0; 
    if (!cygheap->fdtab.not_open(fds[i].fd)) 
    { 
      if (fds[i].events & POLLIN) 
        FD_SET(fds[i].fd, read_fds); 
      if (fds[i].events & POLLOUT) 
        FD_SET(fds[i].fd, write_fds); 
      if (fds[i].events & POLLPRI) 
        FD_SET(fds[i].fd, except_fds); 
    } 
  } 

  int ret = cygwin_select (max_fd + 1, read_fds, write_fds, except_fds, timeout < 0 ? NULL : &tv);

  if (ret > 0) 
    for (unsigned int i = 0; i < nfds; ++i) 
    { 
      if (fds[i].fd < 0) 
        fds[i].revents = POLLNVAL; 
      else if (cygheap->fdtab.not_open(fds[i].fd)) 
        fds[i].revents = POLLHUP; 
      else 
      { 
        if (FD_ISSET(fds[i].fd, read_fds)) 
          fds[i].revents |= POLLIN; 
        if (FD_ISSET(fds[i].fd, write_fds)) 
          fds[i].revents |= POLLOUT; 
        if (FD_ISSET(fds[i].fd, read_fds) && FD_ISSET(fds[i].fd, write_fds)) 
          fds[i].revents |= POLLERR; 
        if (FD_ISSET(fds[i].fd, except_fds)) 
          fds[i].revents |= POLLPRI; 
      } 
    } 

  return ret; 
} 

[-- Attachment #3: patch.cc --]
[-- Type: application/octet-stream, Size: 3370 bytes --]

--- poll.cc	Sun Mar 10 21:52:26 2002
+++ newpoll.cc	Tue Mar 12 00:53:20 2002
@@ -26,7 +26,7 @@ int
 poll (struct pollfd *fds, unsigned int nfds, int timeout)
 {
   int max_fd = 0;
-  fd_set *open_fds, *read_fds, *write_fds, *except_fds;
+  fd_set *read_fds, *write_fds, *except_fds;
   struct timeval tv = { timeout / 1000, (timeout % 1000) * 1000 };
   sigframe thisframe (mainthread);
 
@@ -36,63 +36,55 @@ poll (struct pollfd *fds, unsigned int n
 
   size_t fds_size = howmany(max_fd + 1, NFDBITS) * sizeof (fd_mask);
 
-  open_fds = (fd_set *) alloca (fds_size);
   read_fds = (fd_set *) alloca (fds_size);
   write_fds = (fd_set *) alloca (fds_size);
   except_fds = (fd_set *) alloca (fds_size);
 
-  if (!open_fds || !read_fds || !write_fds || !except_fds)
+  if (!read_fds || !write_fds || !except_fds)
     {
       set_errno (ENOMEM);
       return -1;
     }
 
-  memset (open_fds, 0, fds_size);
   memset (read_fds, 0, fds_size);
   memset (write_fds, 0, fds_size);
   memset (except_fds, 0, fds_size);
 
-  bool valid_fds = false;
-  for (unsigned int i = 0; i < nfds; ++i)
-    if (!cygheap->fdtab.not_open (fds[i].fd))
-      {
-	valid_fds = true;
-	fds[i].revents = 0;
-	FD_SET (fds[i].fd, open_fds);
-	if (fds[i].events & POLLIN)
-	  FD_SET (fds[i].fd, read_fds);
-	if (fds[i].events & POLLOUT)
-	  FD_SET (fds[i].fd, write_fds);
-	if (fds[i].events & POLLPRI)
-	  FD_SET (fds[i].fd, except_fds);
-      }
-      else
-	fds[i].revents = POLLNVAL;
-
-  int ret = 0;
-  if (valid_fds)
-    ret = cygwin_select (max_fd + 1, read_fds, write_fds, except_fds,
-			 timeout < 0 ? NULL : &tv);
-
-  for (unsigned int i = 0; i < nfds; ++i)
-    {
-      if (fds[i].revents == POLLNVAL && ret >= 0)
-	ret++;
-      else if (cygheap->fdtab.not_open(fds[i].fd))
-	fds[i].revents = POLLHUP;
-      else if (ret < 0)
-	fds[i].revents = POLLERR;
-      else
-	{
-	  fds[i].revents = 0;
-	  if (FD_ISSET (fds[i].fd, read_fds))
-	    fds[i].revents |= POLLIN;
-	  if (FD_ISSET (fds[i].fd, write_fds))
-	    fds[i].revents |= POLLOUT;
-	  if (FD_ISSET (fds[i].fd, except_fds))
-	    fds[i].revents |= POLLPRI;
-	}
-    }
+  for (unsigned int i = 0; i < nfds; ++i) 
+  { 
+    fds[i].revents = 0; 
+    if (!cygheap->fdtab.not_open(fds[i].fd)) 
+    { 
+      if (fds[i].events & POLLIN) 
+        FD_SET(fds[i].fd, read_fds); 
+      if (fds[i].events & POLLOUT) 
+        FD_SET(fds[i].fd, write_fds); 
+      if (fds[i].events & POLLPRI) 
+        FD_SET(fds[i].fd, except_fds); 
+    } 
+  } 
+
+  int ret = cygwin_select (max_fd + 1, read_fds, write_fds, except_fds, timeout < 0 ? NULL : &tv);
+
+  if (ret > 0) 
+    for (unsigned int i = 0; i < nfds; ++i) 
+    { 
+      if (fds[i].fd < 0) 
+        fds[i].revents = POLLNVAL; 
+      else if (cygheap->fdtab.not_open(fds[i].fd)) 
+        fds[i].revents = POLLHUP; 
+      else 
+      { 
+        if (FD_ISSET(fds[i].fd, read_fds)) 
+	  fds[i].revents |= POLLIN; 
+	if (FD_ISSET(fds[i].fd, write_fds)) 
+	  fds[i].revents |= POLLOUT; 
+	if (FD_ISSET(fds[i].fd, read_fds) && FD_ISSET(fds[i].fd, write_fds)) 
+	  fds[i].revents |= POLLERR; 
+	if (FD_ISSET(fds[i].fd, except_fds)) 
+	  fds[i].revents |= POLLPRI; 
+      } 
+    } 
 
-  return ret;
-}
+  return ret; 
+} 
\ No newline at end of file

[-- Attachment #4: polltest.c --]
[-- Type: application/octet-stream, Size: 1034 bytes --]

/* gcc -opolltest polltest.c */ 

#include <stdio.h> 
#include <string.h> 
#include <poll.h> 
#include <signal.h> 
#include <unistd.h> 

void callback_alarm(int sig) { return; } 

int main() 
{ 
	struct pollfd p[3]; 

	memset(p, 0, sizeof(struct pollfd) * 3); 

	p[0].fd = -1; 
	p[1].fd = -1; 
	p[2].fd = -1; 

	printf("no descriptor is valid - poll() must wait for 2 seconds\n"); 
	poll(p, 3, 2000); 
	printf("no descriptor is valid - revents for any descriptor must be 0: %d, %d, %d\n", p[0].revents, p[1].revents, p[2].revents); 

	p[2].fd = 0; 
	printf("no descriptor is ready - poll() must return 0: %d\n", poll(p, 3, 500)); 
	printf("no descriptor is ready - revents for any descriptor must be 0: %d, %d, %d\n", p[0].revents, p[1].revents, p[2].revents); 

	signal(SIGALRM, callback_alarm); 

	alarm(1); 

	printf("SIGALRM - poll() must return -1: %d\n", poll(p, 3, 1500)); 
	printf("SIGALRM - revents for any descriptor must be 0: %d, %d, %d\n", p[0].revents, p[1].revents, p[2].revents); 
}

[-- Attachment #5: Type: text/plain, Size: 214 bytes --]

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: poll() bugs and patch

[Corinna Vinschen]

On Wed, Mar 13, 2002 at 02:30:35PM +0100, Boris Schaeling wrote:
> There are several bugs in poll():
> - poll() must return 0 if no descriptor is ready and the timer expires. It
> works correctly if all descriptors are non-negative. Otherwise poll()
> returns the count of negative descriptors which is wrong.
> - The implementation of poll() must call cygwin_select() even if all
> descriptors are invalid. Currently poll() returns immediately - and ignores
> any timer value. The return value is wrong again because of the bug above.
> - If the timer expires revents must be 0 and must not be set to anything
> else like POLLNVAL, POLLHUP or whatever.
> - If cygwin_select() returns -1 revents must be 0 and must not be set to
> POLLERR. Eg. a signal interrupting poll() doesn't mean an error has occured
> for a TCP connection.
> - If an error is pending revents must be set to POLLERR.
> - The implementation of poll() has a local variable called open_fds that
> isn't used. I can't see why this variable exists.
> 
> I've attached three files:
> - newpoll.cc has a new implementation of poll() which fixes the bugs.
> - patch.cc is the patch.
> - polltest.c is a testcase.
> 
> I rebuilt cygwin1.dll with my poll() function so there should be no problems
> when compiling.

Thanks for the patch but that's not quite the way to contribute a
patch.  I'm missing a ChangeLog entry and, worse, we don't have a
copyright assignment from you.  Due to licensing issues we need the
copyright assignment as described on http://cygwin.com/contrib.html
before we can incorporate the patch.  It's bigger than what is
slipping through the cracks as "insignificant".

It would really be nice if you could send us this assignment (which
unfortunately has to be send via snail mail so this takes some time)
and then resubmit your patch to cygwin-patches@cygwin.com.

Oh, could you please rename the patch file?  It's somewhat
irritating to have a *.cc suffix for a patch file. `poll.diff' or
poll.patch' would be nice...

Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Developer                                mailto:cygwin@cygwin.com
Red Hat, Inc.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

AW: poll() bugs and patch

[Boris Schaeling]

> -----Ursprüngliche Nachricht-----
> Von: cygwin-owner@cygwin.com [mailto:cygwin-owner@cygwin.com]Im Auftrag
> von Corinna Vinschen
> Gesendet: Mittwoch, 13. März 2002 15:06
> An: cygwin@cygwin.com
> Betreff: Re: poll() bugs and patch

> [...]
> Thanks for the patch but that's not quite the way to contribute a
> patch.  I'm missing a ChangeLog entry and, worse, we don't have a
> copyright assignment from you.  Due to licensing issues we need the
> copyright assignment as described on http://cygwin.com/contrib.html
> before we can incorporate the patch.  It's bigger than what is
> slipping through the cracks as "insignificant".

Okay. I thought this is only a small patch and were a little bit lazy. I'll
see what I can do. :)

> It would really be nice if you could send us this assignment (which
> unfortunately has to be send via snail mail so this takes some time)

Can I fax the assignment?

Boris


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: poll() bugs and patch

[Corinna Vinschen]

On Wed, Mar 13, 2002 at 04:28:24PM +0100, Boris Schaeling wrote:
> > -----Ursprüngliche Nachricht-----
> > Von: cygwin-owner@cygwin.com [mailto:cygwin-owner@cygwin.com]Im Auftrag
> > von Corinna Vinschen
> > Gesendet: Mittwoch, 13. März 2002 15:06
> > An: cygwin@cygwin.com
> > Betreff: Re: poll() bugs and patch
> 
> > [...]
> > Thanks for the patch but that's not quite the way to contribute a
> > patch.  I'm missing a ChangeLog entry and, worse, we don't have a
> > copyright assignment from you.  Due to licensing issues we need the
> > copyright assignment as described on http://cygwin.com/contrib.html
> > before we can incorporate the patch.  It's bigger than what is
> > slipping through the cracks as "insignificant".
> 
> Okay. I thought this is only a small patch and were a little bit lazy. I'll
> see what I can do. :)
> 
> > It would really be nice if you could send us this assignment (which
> > unfortunately has to be send via snail mail so this takes some time)
> 
> Can I fax the assignment?

Uhm, I'm still not sure about this.  Perhaps Chris can enlighten
us.  You're not the first one asking...

Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Developer                                mailto:cygwin@cygwin.com
Red Hat, Inc.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

dll call and fork

[Stephane Corbe]

Hello,

    If I fork my programm,
    the child died when it call a function in a DLL. (Cygwin 1.3.10-1 on
NT4)

void dll_call(int i)
{
     printf("in dll_call %d\n", i);
 }

...

   printf("begin");
   switch (pid=fork())
   {
        case 0 :
            printf("child is alive\n");
            dll_call(pid);
            printf("after dll_call\n");
            break;

        case -1 :
            printf ("fork error\n");
            break;

        default:
            printf ("parent is alive\n");
            dll_call(pid);
            while (1);  // or sleep; gets, ... same result
    }


Output is :

begin
child_is_alive
parent is alive
in dll_call 0
/* nothing else */


Did you see this before ?

    Stephane




--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: poll() bugs and patch

[Christopher Faylor]

On Wed, Mar 13, 2002 at 04:36:41PM +0100, Corinna Vinschen wrote:
>Uhm, I'm still not sure about this.  Perhaps Chris can enlighten
>us.  You're not the first one asking...

Actually, I'm inclined to just let this one in.  There isn't a lot of
new code, it's mainly just rearranged.

FWIW, faxing is not enough.  It has to be mailed.

cgf

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: poll() bugs and patch

[Corinna Vinschen]

On Wed, Mar 13, 2002 at 11:39:24AM -0500, Chris Faylor wrote:
> On Wed, Mar 13, 2002 at 04:36:41PM +0100, Corinna Vinschen wrote:
> >Uhm, I'm still not sure about this.  Perhaps Chris can enlighten
> >us.  You're not the first one asking...
> 
> Actually, I'm inclined to just let this one in.  There isn't a lot of
> new code, it's mainly just rearranged.

Ok with me.  However, I'll not take that patch w/o a ChangeLog entry.
Please create one, Boris.

Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Developer                                mailto:cygwin@cygwin.com
Red Hat, Inc.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

AW: poll() bugs and patch

[Boris Schaeling]

[-- Attachment #1: Type: text/plain, Size: 580 bytes --]



> -----Ursprüngliche Nachricht-----
> Von: cygwin-owner@cygwin.com [mailto:cygwin-owner@cygwin.com]Im Auftrag
> von Corinna Vinschen
> Gesendet: Mittwoch, 13. März 2002 18:14
> An: cygwin@cygwin.com
> Betreff: Re: poll() bugs and patch

> [...]
> > Actually, I'm inclined to just let this one in.  There isn't a lot of
> > new code, it's mainly just rearranged.
>
> Ok with me.  However, I'll not take that patch w/o a ChangeLog entry.
> Please create one, Boris.

Here they are:
- change.log
- poll.patch
- newpoll.cc (new implementation)
- polltest.c (testcase)

HTH,
Boris

[-- Attachment #2: change.log --]
[-- Type: application/octet-stream, Size: 144 bytes --]

2002-03-13  Boris Schaeling  <boriss@web.de> 

	* poll.cc: variable open_fds removed, rearranged 
	and added code to fix settings of revents 

[-- Attachment #3: poll.patch --]
[-- Type: application/octet-stream, Size: 3370 bytes --]

--- poll.cc	Sun Mar 10 21:52:26 2002
+++ newpoll.cc	Tue Mar 12 00:53:20 2002
@@ -26,7 +26,7 @@ int
 poll (struct pollfd *fds, unsigned int nfds, int timeout)
 {
   int max_fd = 0;
-  fd_set *open_fds, *read_fds, *write_fds, *except_fds;
+  fd_set *read_fds, *write_fds, *except_fds;
   struct timeval tv = { timeout / 1000, (timeout % 1000) * 1000 };
   sigframe thisframe (mainthread);
 
@@ -36,63 +36,55 @@ poll (struct pollfd *fds, unsigned int n
 
   size_t fds_size = howmany(max_fd + 1, NFDBITS) * sizeof (fd_mask);
 
-  open_fds = (fd_set *) alloca (fds_size);
   read_fds = (fd_set *) alloca (fds_size);
   write_fds = (fd_set *) alloca (fds_size);
   except_fds = (fd_set *) alloca (fds_size);
 
-  if (!open_fds || !read_fds || !write_fds || !except_fds)
+  if (!read_fds || !write_fds || !except_fds)
     {
       set_errno (ENOMEM);
       return -1;
     }
 
-  memset (open_fds, 0, fds_size);
   memset (read_fds, 0, fds_size);
   memset (write_fds, 0, fds_size);
   memset (except_fds, 0, fds_size);
 
-  bool valid_fds = false;
-  for (unsigned int i = 0; i < nfds; ++i)
-    if (!cygheap->fdtab.not_open (fds[i].fd))
-      {
-	valid_fds = true;
-	fds[i].revents = 0;
-	FD_SET (fds[i].fd, open_fds);
-	if (fds[i].events & POLLIN)
-	  FD_SET (fds[i].fd, read_fds);
-	if (fds[i].events & POLLOUT)
-	  FD_SET (fds[i].fd, write_fds);
-	if (fds[i].events & POLLPRI)
-	  FD_SET (fds[i].fd, except_fds);
-      }
-      else
-	fds[i].revents = POLLNVAL;
-
-  int ret = 0;
-  if (valid_fds)
-    ret = cygwin_select (max_fd + 1, read_fds, write_fds, except_fds,
-			 timeout < 0 ? NULL : &tv);
-
-  for (unsigned int i = 0; i < nfds; ++i)
-    {
-      if (fds[i].revents == POLLNVAL && ret >= 0)
-	ret++;
-      else if (cygheap->fdtab.not_open(fds[i].fd))
-	fds[i].revents = POLLHUP;
-      else if (ret < 0)
-	fds[i].revents = POLLERR;
-      else
-	{
-	  fds[i].revents = 0;
-	  if (FD_ISSET (fds[i].fd, read_fds))
-	    fds[i].revents |= POLLIN;
-	  if (FD_ISSET (fds[i].fd, write_fds))
-	    fds[i].revents |= POLLOUT;
-	  if (FD_ISSET (fds[i].fd, except_fds))
-	    fds[i].revents |= POLLPRI;
-	}
-    }
+  for (unsigned int i = 0; i < nfds; ++i) 
+  { 
+    fds[i].revents = 0; 
+    if (!cygheap->fdtab.not_open(fds[i].fd)) 
+    { 
+      if (fds[i].events & POLLIN) 
+        FD_SET(fds[i].fd, read_fds); 
+      if (fds[i].events & POLLOUT) 
+        FD_SET(fds[i].fd, write_fds); 
+      if (fds[i].events & POLLPRI) 
+        FD_SET(fds[i].fd, except_fds); 
+    } 
+  } 
+
+  int ret = cygwin_select (max_fd + 1, read_fds, write_fds, except_fds, timeout < 0 ? NULL : &tv);
+
+  if (ret > 0) 
+    for (unsigned int i = 0; i < nfds; ++i) 
+    { 
+      if (fds[i].fd < 0) 
+        fds[i].revents = POLLNVAL; 
+      else if (cygheap->fdtab.not_open(fds[i].fd)) 
+        fds[i].revents = POLLHUP; 
+      else 
+      { 
+        if (FD_ISSET(fds[i].fd, read_fds)) 
+	  fds[i].revents |= POLLIN; 
+	if (FD_ISSET(fds[i].fd, write_fds)) 
+	  fds[i].revents |= POLLOUT; 
+	if (FD_ISSET(fds[i].fd, read_fds) && FD_ISSET(fds[i].fd, write_fds)) 
+	  fds[i].revents |= POLLERR; 
+	if (FD_ISSET(fds[i].fd, except_fds)) 
+	  fds[i].revents |= POLLPRI; 
+      } 
+    } 
 
-  return ret;
-}
+  return ret; 
+} 
\ No newline at end of file

[-- Attachment #4: polltest.c --]
[-- Type: application/octet-stream, Size: 1033 bytes --]

/* gcc -opolltest polltest.c */ 

#include <stdio.h> 
#include <string.h> 
#include <poll.h> 
#include <signal.h> 
#include <unistd.h> 

void callback_alarm(int sig) { return; } 

int main() 
{ 
	struct pollfd p[3]; 

	memset(p, 0, sizeof(struct pollfd) * 3); 

	p[0].fd = -1; 
	p[1].fd = -1; 
	p[2].fd = -1; 

	printf("no descriptor is valid - poll() must wait for 2 seconds\n"); 
	poll(p, 3, 2000); 
	printf("no descriptor is valid - revents for any descriptor must be 0: %d, %d, %d\n", p[0].revents, p[1].revents, p[2].revents); 

	p[2].fd = 0; 
	printf("no descriptor is ready - poll() must return 0: %d\n", poll(p, 3, 500)); 
	printf("no descriptor is ready - revents for any descriptor must be 0: %d, %d, %d\n", p[0].revents, p[1].revents, p[2].revents); 

	signal(SIGALRM, callback_alarm); 

	alarm(1); 

	printf("SIGALRM - poll() must return -1: %d\n", poll(p, 3, 1500)); 
	printf("SIGALRM - revents for any descriptor must be 0: %d, %d, %d\n", p[0].revents, p[1].revents, p[2].revents); 
} 

[-- Attachment #5: newpoll.cc --]
[-- Type: application/octet-stream, Size: 2473 bytes --]

/* poll.cc. Implements poll(2) via usage of select(2) call.

   Copyright 2000, 2001 Red Hat, Inc.

   This file is part of Cygwin.

   This software is a copyrighted work licensed under the terms of the
   Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
   details. */

#include "winsup.h"
#include <sys/time.h>
#include <sys/poll.h>
#include <errno.h>
#include <stdlib.h>
#include "security.h"
#include "fhandler.h"
#include "path.h"
#include "dtable.h"
#include "cygerrno.h"
#include "cygheap.h"
#include "sigproc.h"

extern "C"
int
poll (struct pollfd *fds, unsigned int nfds, int timeout)
{
  int max_fd = 0;
  fd_set *read_fds, *write_fds, *except_fds;
  struct timeval tv = { timeout / 1000, (timeout % 1000) * 1000 };
  sigframe thisframe (mainthread);

  for (unsigned int i = 0; i < nfds; ++i)
    if (fds[i].fd > max_fd)
      max_fd = fds[i].fd;

  size_t fds_size = howmany(max_fd + 1, NFDBITS) * sizeof (fd_mask);

  read_fds = (fd_set *) alloca (fds_size);
  write_fds = (fd_set *) alloca (fds_size);
  except_fds = (fd_set *) alloca (fds_size);

  if (!read_fds || !write_fds || !except_fds)
    {
      set_errno (ENOMEM);
      return -1;
    }

  memset (read_fds, 0, fds_size);
  memset (write_fds, 0, fds_size);
  memset (except_fds, 0, fds_size);

  for (unsigned int i = 0; i < nfds; ++i) 
  { 
    fds[i].revents = 0; 
    if (!cygheap->fdtab.not_open(fds[i].fd)) 
    { 
      if (fds[i].events & POLLIN) 
        FD_SET(fds[i].fd, read_fds); 
      if (fds[i].events & POLLOUT) 
        FD_SET(fds[i].fd, write_fds); 
      if (fds[i].events & POLLPRI) 
        FD_SET(fds[i].fd, except_fds); 
    } 
  } 

  int ret = cygwin_select (max_fd + 1, read_fds, write_fds, except_fds, timeout < 0 ? NULL : &tv);

  if (ret > 0) 
    for (unsigned int i = 0; i < nfds; ++i) 
    { 
      if (fds[i].fd < 0) 
        fds[i].revents = POLLNVAL; 
      else if (cygheap->fdtab.not_open(fds[i].fd)) 
        fds[i].revents = POLLHUP; 
      else 
      { 
        if (FD_ISSET(fds[i].fd, read_fds)) 
          fds[i].revents |= POLLIN; 
        if (FD_ISSET(fds[i].fd, write_fds)) 
          fds[i].revents |= POLLOUT; 
        if (FD_ISSET(fds[i].fd, read_fds) && FD_ISSET(fds[i].fd, write_fds)) 
          fds[i].revents |= POLLERR; 
        if (FD_ISSET(fds[i].fd, except_fds)) 
          fds[i].revents |= POLLPRI; 
      } 
    } 

  return ret; 
}

[-- Attachment #6: Type: text/plain, Size: 214 bytes --]

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: poll() bugs and patch

[Corinna Vinschen]

On Wed, Mar 13, 2002 at 10:20:30PM +0100, Boris Schaeling wrote:
> > Ok with me.  However, I'll not take that patch w/o a ChangeLog entry.
> > Please create one, Boris.
> 
> Here they are:
> - change.log
> - poll.patch
> - newpoll.cc (new implementation)
> - polltest.c (testcase)

Thanks for the patch.  I've applied it with some changes.  Sorry for
being picky but your indenting wasn't GNU coding style conformant.
It's not

  if ()
  {
    foo;
  }

but

  if ()
    {
      foo;
    }

I've fixed that for now.  Your ChangeLog isn't correct either but
I've fixed that too:

Not

        * poll.cc: variable open_fds removed, rearranged
	and added code to fix settings of revents

but

        * poll.cc: Remove variable open_fds, rearrange and add code
	to fix settings of revents.

Present tense, full stop.

Check out the next developers snapshot or update from CVS to see
your patch incorporated.

Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Developer                                mailto:cygwin@cygwin.com
Red Hat, Inc.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

AW: poll() bugs and patch

[Boris Schaeling]

> -----Ursprüngliche Nachricht-----
> Von: cygwin-owner@cygwin.com [mailto:cygwin-owner@cygwin.com]Im Auftrag
> von Corinna Vinschen
> Gesendet: Donnerstag, 14. März 2002 13:16
> An: cygwin
> Betreff: Re: poll() bugs and patch

> [...]
> Thanks for the patch.  I've applied it with some changes.  Sorry for
> [...]
> Check out the next developers snapshot or update from CVS to see
> your patch incorporated.

Alright, thank you, Corinna!

Boris


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Latest poll() hangs again (was Re: poll() bugs and patch)

[Jason Tishler]

On Thu, Mar 14, 2002 at 01:15:54PM +0100, Corinna Vinschen wrote:
> Thanks for the patch.  I've applied it with some changes.

I was concerned when I first saw this patch.  Unfortunately, I just
tried it and my concerns were realized.

This patch causes poll() to hang again when only an invalid file
descriptor is specified.  See the following for a test case:

    http://sources.redhat.com/ml/cygwin-patches/2001-q3/msg00109.html

Please revert this patch or modify it so that poll() does not hang.

Thanks,
Jason

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: Latest poll() hangs again (was Re: poll() bugs and patch)

[Corinna Vinschen]

On Fri, Mar 15, 2002 at 03:01:21PM -0500, Jason Tishler wrote:
> On Thu, Mar 14, 2002 at 01:15:54PM +0100, Corinna Vinschen wrote:
> > Thanks for the patch.  I've applied it with some changes.
> 
> I was concerned when I first saw this patch.  Unfortunately, I just
> tried it and my concerns were realized.
> 
> This patch causes poll() to hang again when only an invalid file
> descriptor is specified.  See the following for a test case:
> 
>     http://sources.redhat.com/ml/cygwin-patches/2001-q3/msg00109.html
> 
> Please revert this patch or modify it so that poll() does not hang.

I don't think reverting is the way to go.  One of the problems
was that cygwin_select isn't called at all if all fd's are
invalid, even if a timeout value is given.  Boris patch fixed
that.  Unfortunately the problem you're describing is a border
case which wasn't handled correctly by your patch as well, AFAICS.
The correct solution would be to return immediately (aka
cygwin_select isn't called) only if all fd's are invalid AND
the timeout value is set to INFINITE (-1).

Unfortunately SUSv2 doesn't handle that special case.  That's
a pity.

However, we could revert to handle invalid_fds again and then call
cygwin_select only if timeout != -1.

Comments?  Boris?  Jason?  Anybody else?

Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Developer                                mailto:cygwin@cygwin.com
Red Hat, Inc.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: Latest poll() hangs again (was Re: poll() bugs and patch)

[Christopher Faylor]

On Fri, Mar 15, 2002 at 10:19:40PM +0100, Corinna Vinschen wrote:
>On Fri, Mar 15, 2002 at 03:01:21PM -0500, Jason Tishler wrote:
>> On Thu, Mar 14, 2002 at 01:15:54PM +0100, Corinna Vinschen wrote:
>> > Thanks for the patch.  I've applied it with some changes.
>> 
>> I was concerned when I first saw this patch.  Unfortunately, I just
>> tried it and my concerns were realized.
>> 
>> This patch causes poll() to hang again when only an invalid file
>> descriptor is specified.  See the following for a test case:
>> 
>>     http://sources.redhat.com/ml/cygwin-patches/2001-q3/msg00109.html
>> 
>> Please revert this patch or modify it so that poll() does not hang.
>
>I don't think reverting is the way to go.  One of the problems
>was that cygwin_select isn't called at all if all fd's are
>invalid, even if a timeout value is given.  Boris patch fixed
>that.  Unfortunately the problem you're describing is a border
>case which wasn't handled correctly by your patch as well, AFAICS.
>The correct solution would be to return immediately (aka
>cygwin_select isn't called) only if all fd's are invalid AND
>the timeout value is set to INFINITE (-1).
>
>Unfortunately SUSv2 doesn't handle that special case.  That's
>a pity.
>
>However, we could revert to handle invalid_fds again and then call
>cygwin_select only if timeout != -1.
>
>Comments?  Boris?  Jason?  Anybody else?

How does linux handle these cases?

cgf

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

Re: Latest poll() hangs again (was Re: poll() bugs and patch)

[Corinna Vinschen]

On Fri, Mar 15, 2002 at 04:33:21PM -0500, Chris Faylor wrote:
> On Fri, Mar 15, 2002 at 10:19:40PM +0100, Corinna Vinschen wrote:
> >On Fri, Mar 15, 2002 at 03:01:21PM -0500, Jason Tishler wrote:
> >> Please revert this patch or modify it so that poll() does not hang.
> >
> >I don't think reverting is the way to go.  One of the problems
> >[...]
> >Unfortunately SUSv2 doesn't handle that special case.  That's
> >a pity.
> >
> >However, we could revert to handle invalid_fds again and then call
> >cygwin_select only if timeout != -1.
> >
> >Comments?  Boris?  Jason?  Anybody else?
> 
> How does linux handle these cases?

Just tested.  It returns immediately if all fd's are invalid,
regardless of the timeout value.  So there's actually no need
to call cygwin_select if all fd's are invalid.  I'm not quite
sure about the other border cases, Boris is talking about though.
This requires further testing.  Sigh, I'd hoped that somebody
else is doing that this time...

I'm inclined to revert the patch and ask Boris to investigate
the other cases to come up with a a new patch which still handles
the "all fd's are invalid, don't call cygwin_select" correctly...

Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Developer                                mailto:cygwin@cygwin.com
Red Hat, Inc.

--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/

AW: Latest poll() hangs again (was Re: poll() bugs and patch)

[Boris Schaeling]

> -----Ursprüngliche Nachricht-----
> Von: cygwin-owner@cygwin.com [mailto:cygwin-owner@cygwin.com]Im Auftrag
> von Corinna Vinschen
> Gesendet: Freitag, 15. März 2002 23:28
> An: cygwin@cygwin.com
> Betreff: Re: Latest poll() hangs again (was Re: poll() bugs and patch)

> [...]
> I'm inclined to revert the patch and ask Boris to investigate
> the other cases to come up with a a new patch which still handles
> the "all fd's are invalid, don't call cygwin_select" correctly...

That's what I'm doing currently. :)
I'll send the modified poll() asap.

Boris


--
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple
Bug reporting:         http://cygwin.com/bugs.html
Documentation:         http://cygwin.com/docs.html
FAQ:                   http://cygwin.com/faq/