Re: 1.1.4 select and poll false positive past 32 descriptors

Re: 1.1.4 select and poll false positive past 32 descriptors

[Ted Soo-Hoo]

Chris Faylor wrote:

> From the linux man page on select:
>        n is the highest-numbered descriptor in any of  the  three
>        sets, plus 1.

I understand everything you're telling me. Please let me explain
some more (citing source) and perhaps this will clarify my point.
It's possible I'm way off, but we're miscommunicating right now.

> What is allocated in select.cc reflects the maxiumum number of fds
> that can be operated on.  It's essentially a bit mask.  I don't know
> why you are mentioning this.  If your maximum fd is something like
> 64, then the fd_set's allocated should be at least that large.

I think that was its intent. I suspect that is not what it's doing.
The strace output for recv_fd_max = 42 (so select was given 43) is:

18931 1299283 [main] selectbug 1327 select_stuff::wait: n 2, ms 4294967295

I agree that the select.cc allocation should hold the specified fds,
but I think it's allocating based on that "n 2". I'll explain later.

I'm not setting FD_SETSIZE because the default of 64 is enough here.
You can see from the printing based on sizeof that it's eight bytes.

> I don't know what "the expected value to copy out" might be or what
> you mean by "not work too hard beyond that".

Copy out refers to the copy_fdset at "out:" below to return the fd_set.
The "expected value to copy out" is the number of bits, 43 in the test.

Working too hard refers to the complex count loop in the sources below.
Just looking superficially, it looks like it's counting something with
wait objects and "active fds" in mind. It then allocates based on that.
My guess is that in terms of filling out the w4 array, it's doing fine,
but it then uses the same value later to allocate and return an fd_set.

> I think what you are reporting is that the winsock version of select
> only handles 32 elements.  That's an unfortunate limitation.  It would
> probably be possible to work around this by forking multiple threads to
> handle multiple winsock selects but this would be a fairly major change
> and I am not aware of anyone contemplating this.  Sorry.

I'm reporting that if I use the workaround described, it handles 200.
I think there may be a problem in the allocation code that counts too
few bits and keeps it from reporting its returned bitmasks correctly.

Let me give some snippets from the source code, along with commentary:

These macros I think are to allocate an fd_set based on needed bits:

#define unix_howmany(x,y) (((x)+((y)-1))/(y))

#define sizeof_fd_set(n) \
  ((unsigned) (NULL_fd_set->fds_bits + unix_howmany((n), UNIX_NFDBITS)))

#define allocfd_set(n) ((fd_set *) alloca (sizeof_fd_set (n)))

I'm not sure what w4 is for, but it seems to have to do with waiting:

  HANDLE w4[MAXIMUM_WAIT_OBJECTS];

Here it starts referring to "active fds" which gets me a bit worried:

  /* Loop through the select chain, starting up anything appropriate and
     counting the number of active fds. */
  while ((s = s->next))
    {
      if (!s->startup (s, this))
        {
          __seterrno ();
          return -1;
        }
      if (s->h == NULL)
        continue;
      for (int i = 1; i < m; i++)
        if (w4[i] == s->h)
          goto next_while;
      w4[m++] = s->h;
  next_while:
      continue;
    }

Here it connects n with m whereas maybe it should have counted an n:

  int n = m - 1;

There are a number of uses of m and of n in the code. Let me show n.
Also, I think this is the strace debug_printf that said "n 2" above:

  /* Allocate some fd_set structures using the number of fds as a guide. */
  fd_set *r = allocfd_set (n);
  fd_set *w = allocfd_set (n);
  fd_set *e = allocfd_set (n);
  UNIX_FD_ZERO (r, n);
  UNIX_FD_ZERO (w, n);
  UNIX_FD_ZERO (e, n);
  debug_printf ("n %d, ms %u", n, ms);

out:
  copyfd_set (readfds, r, n);

I think what's happening is it's coming up with a very low n, and the
macros above are having it allocate the minimum 32-bit fds internally
and have the copyfd_set write that much back into the user's bitmasks
thus leaving the additional bitmasks for more than 32 bits untouched.

Since those bits were one-initialized to be tested, they come back as
false-positive results unless they are cleared. Thus I'm getting back

01 00 00 00 ff 07 00 00

and not

01 00 00 00 00 00 00 00

when I simply have stdin be ready.

I think the successful second result is because it runs sel.poll not
sel.wait (which is where the code is that I'm thinking has a bad n).

I hope this clarifies things. I might still be off but I can get 200
select bits back on my workaround and I'm wondering if it would work
without my workaround if the bit-counting was changed. I don't know.

It would be great if select would get the rest of the way and not be
stuck at 32. Even before it was dynamic, wasn't the limit set at 64?

Regards,
Ted

--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com

Re: 1.1.4 select and poll false positive past 32 descriptors

[Chris Faylor]

On Tue, Aug 08, 2000 at 07:54:23PM -0400, Ted Soo-Hoo wrote:
>Chris Faylor wrote:
>
>> From the linux man page on select:
>>        n is the highest-numbered descriptor in any of  the  three
>>        sets, plus 1.
>
>I understand everything you're telling me. Please let me explain
>some more (citing source) and perhaps this will clarify my point.
>It's possible I'm way off, but we're miscommunicating right now.
>
>> What is allocated in select.cc reflects the maxiumum number of fds
>> that can be operated on.  It's essentially a bit mask.  I don't know
>> why you are mentioning this.  If your maximum fd is something like
>> 64, then the fd_set's allocated should be at least that large.
>
>I think that was its intent. I suspect that is not what it's doing.
>The strace output for recv_fd_max = 42 (so select was given 43) is:
>
>18931 1299283 [main] selectbug 1327 select_stuff::wait: n 2, ms 4294967295
>
>I agree that the select.cc allocation should hold the specified fds,
>but I think it's allocating based on that "n 2". I'll explain later.

The 'n == 2' has to do with the number of things that the
WaitForMultipleObjects is waiting for.  In the case that you cited, that
is probably "two threads".  This is completely unrelated to the argument
to the select call.

>I'm not setting FD_SETSIZE because the default of 64 is enough here.
>You can see from the printing based on sizeof that it's eight bytes.
>
>> I don't know what "the expected value to copy out" might be or what
>> you mean by "not work too hard beyond that".
>
>Copy out refers to the copy_fdset at "out:" below to return the fd_set.
>The "expected value to copy out" is the number of bits, 43 in the test.
>
>Working too hard refers to the complex count loop in the sources below.
>Just looking superficially, it looks like it's counting something with
>wait objects and "active fds" in mind. It then allocates based on that.
>My guess is that in terms of filling out the w4 array, it's doing fine,
>but it then uses the same value later to allocate and return an fd_set.

Yep.  You're right.  You've identified a bug.

Thanks for the extra details.  I'll fix this.

cgf

--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com

Re: 1.1.4 select and poll false positive past 32 descriptors

[Ted Soo-Hoo]

Chris Faylor wrote:

> Thanks for the extra details.  I'll fix this.

Thanks for taking a look at select. Here are some other comments.
I've looked at the source code some more and also tried to learn a
little about Win32 programming. I still don't understand all of what's
going on in the select.cc code so please do as you like with these.
I don't really need an answer. I'll just be pleased if any of this helps.

  HANDLE w4[MAXIMUM_WAIT_OBJECTS];
This constant is 64 whenever I've seen it. The loop in select.cc is
not doing a check for overrunning the array. I hope users who try
wide selects won't cause that. I don't quite understand the w4 use.
I know that in my test where I set a lot of read bits, m was small.

I was also wondering if poll() was implemented with select() as is
sometimes done, so had a look at poll.cc. In this file my concern
is about width of the fd_set being used. The select.cc code tries
to dynamically adapt that by allocfd_set()  to the needed bit-width.
If poll.cc is using the standard 64-bit fd_set (and FD_ZERO, etc.)
it may write past the allocated bits if it gets a descriptor over 63.

Still, even if poll has a limit for awhile it would be nice if a really
wide select could be supported, as the code seems to try to do.

Thanks once again for your help!

Regards,
Ted


--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com

Re: 1.1.4 select and poll false positive past 32 descriptors

[Chris Faylor]

On Tue, Aug 08, 2000 at 05:21:49PM -0400, Ted Soo-Hoo wrote:
>The idea for polling comes from select.cc code which goes down a
>different path for that case.  I also ran under strace and saw that a
>pended operation bases its returned size on a value n which is always
>small, e.g.  1 or 2 and not the expected value to copy out.  That value
>should be based on the maximum file descriptor given by the user, but
>not work too hard beyond that.

From the linux man page on select:
       n is the highest-numbered descriptor in any of  the  three
       sets, plus 1.

What is allocated in select.cc reflects the maxiumum number of fds
that can be operated on.  It's essentially a bit mask.  I don't know
why you are mentioning this.  If your maximum fd is something like
64, then the fd_set's allocated should be at least that large.

I don't know what "the expected value to copy out" might be or what
you mean by "not work too hard beyond that".

>A possible fix would be to change the loop that does the counting to
>count n less conditionally.  Just a thought.  I don't understand all
>the code.

I think what you are reporting is that the winsock version of select
only handles 32 elements.  That's an unfortunate limitation.  It would
probably be possible to work around this by forking multiple threads to
handle multiple winsock selects but this would be a fairly major change
and I am not aware of anyone contemplating this.  Sorry.

cgf

--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com

1.1.4 select and poll false positive past 32 descriptors

[Ted Soo-Hoo]

Hello,

When invoked in a way that may pend they fail
to update any result data past descriptors 0-31.
This typically means you get false positive bits.

NT 4.0 + cygwin1.dll 1.1.4 as built 2000-08-03.

It looks from the sources like the intent was to
support a large variable number of descriptors,
not (as may have been once) a fixed maximum.
If that's not so, the rest of this might not apply.

Sample output of test program with 40 sockets
plus three for the stdin, stdout, and stderr fds:

Press Enter to run the test
recv_fd_max = 42
read_fds before select that pends:
f9 ff ff ff ff 07 00 00
read_fds after select that pends:
01 00 00 00 ff 07 00 00
read_fds before select that polls:
f9 ff ff ff ff 07 00 00
read_fds after select that polls:
01 00 00 00 00 00 00 00

The pending select set the first read_fds word
but didn't clear the second 32-bit word. If I use
a no-time-timeout I get expected read_fds bits.
Nothing is talking to the sockets so they're 0s.

To return descriptors as ready for reading that
are not really ready will hang programs that do
reads based on select saying it's OK to do so.

A workaround for the problem is to use both of
these selects, one to pend and one to get bits.
I've tested up to around 200 this way, using a
larger FD_SETSIZE when compiling of course.

The idea for polling comes from select.cc code
which goes down a different path for that case.
I also ran under strace and saw that a pended
operation bases its returned size on a value n
which is always small, e.g. 1 or 2 and not the
expected value to copy out. That value should
be based on the maximum file descriptor given
by the user, but not work too hard beyond that.

A possible fix would be to change the loop that
does the counting to count n less conditionally.
Just a thought. I don't understand all the code.

By the way, an attempt to wait on a large poll
array suffers a similar fate -- a scan for active
elements finds the one with file descriptor 32
active when it shouldn't be. Probably a leftover
POLLIN came back on all the high descriptors.

Test program for select:

#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <netdb.h>
#include <sys/time.h>
#include <unistd.h>

#define SOCKET_MAX 40

main() {
    int count;
    int recv_fd, recv_fd_max;
    int status;
    struct sockaddr_in addr;
    struct protoent *proto_ptr;
    fd_set recv_fds;
    fd_set zero_fds;
    fd_set read_fds, write_fds, except_fds;
    struct timeval tv;

    recv_fd_max = 0;
    FD_ZERO(&recv_fds);
    FD_ZERO(&zero_fds);

    addr.sin_family = PF_INET;
    proto_ptr = getprotobyname("udp");
    if (proto_ptr == 0) {
	printf("Could not find protocol number for udp\n");
	exit(1);
    }

    addr.sin_addr.s_addr = inet_addr("127.0.0.1");
    addr.sin_port = htons(0);
    for (count = 0; count < SOCKET_MAX; count++) {
	recv_fd = socket(AF_INET, SOCK_DGRAM, proto_ptr->p_proto);
	status = bind(recv_fd, (struct sockaddr *) &addr,
	    sizeof(struct sockaddr));
	if (status == -1) {
	    perror("Could not bind");
	    exit(1);
	}
	FD_SET(recv_fd, &recv_fds);
	if (recv_fd > recv_fd_max) {
	    recv_fd_max = recv_fd;
	}
    }

    FD_SET(0, &recv_fds);
    printf("Press Enter to run the test\n");

    memcpy(&read_fds, &recv_fds, sizeof(fd_set));
    memcpy(&write_fds, &zero_fds, sizeof(fd_set));
    memcpy(&except_fds, &zero_fds, sizeof(fd_set));

    printf("recv_fd_max = %d\n", recv_fd_max);
    print_fds("read_fds before select that pends:\n", &read_fds);

    status = select(recv_fd_max + 1,
	    &read_fds, &write_fds, &except_fds, NULL);
    if (status == -1) {
	printf("Select failed\n");
	exit(1);
    }

    print_fds("read_fds after select that pends:\n", &read_fds);

    memcpy(&read_fds, &recv_fds, sizeof(fd_set));
    memcpy(&write_fds, &zero_fds, sizeof(fd_set));
    memcpy(&except_fds, &zero_fds, sizeof(fd_set));

    print_fds("read_fds before select that polls:\n", &read_fds);

    tv.tv_sec = 0;
    tv.tv_usec = 0;
    status = select(recv_fd_max + 1,
	    &read_fds, &write_fds, &except_fds, &tv);
    if (status == -1) {
	printf("Select failed\n");
	exit(1);
    }

    print_fds("read_fds after select that polls:\n", &read_fds);
}

print_fds(label, fds)
    char *label;
    fd_set *fds;
{
    int count;

    printf("%s", label);
    for (count = 0; count < sizeof(fd_set); count++) {
	printf("%02x ", * ((unsigned char *) fds + count));
    }
    printf("\n");
}

Thank you!

Regards,
Ted

--
Want to unsubscribe from this list?
Send a message to cygwin-unsubscribe@sourceware.cygnus.com