On 07/06/2011 03:58 PM, Mladen Turk wrote:
> On 07/06/2011 02:52 PM, Eric van der Maarel wrote:
>> Hi,
>>
>> Recently I proposed a fix for the select version of apr_pollset_poll
>> implementation, that prevents cpu going to 100% when all clients have
>> disconnected
>> (http://mail-archives.apache.org/mod_mbox/apr-dev/201106.mbox/browser). It actually provides the select impl. of poll with the ability to issue APR_POLLHUPP return events.
>>
>> Jeff, could you please have a look at it and post any remarks you might
>> have.
>
> If Jeff can't, I can ;)
Ok. Just thought it'd be more Jeff's cup of tea. Thanks.

> 1. One byte is enough; eg. recv(s,&c, 1, MSG_PEEK)
sure

> 2. Not sure that other platforms *always* set POLLHUP for
>      half closed sockets.
It doesn't always set POLLHUP. When socket is in accepted state, 
select() keeps it in the read set and reading with recv() returns 0 
graceful disconnect, possibly sudden disconnect) then POLLHUP is set. 
Seems appropriate to me. When recv() result <0 POLLERR flag is set.

Part of my concern is recv() can return 0 when a socket is ready for 
accept(). We just cannot detect this situation proper. My assumption is 
here that it cannot happen with sockets in the poller: they are in 
accept()-ed state already.

> 3. Missing an actual .diff -u file
added

>
>> How do I get about to get a patch like this adopted in the apr source?
>>
>
> Create a proper patch. It can be a while until someone
> does your homework :)
the diff enough?

Thanks,
Eric

>
>
>
> Regards

diff -u select.c.orig select.c
--- select.c.orig	2009-10-02 18:24:00.000000000 +0200
+++ select.c	2011-07-07 14:55:29.000000000 +0200
@@ -149,6 +149,14 @@
              break;
          }
          if (FD_ISSET(fd, &readset)) {
+            char c;
+            int r = recv(fd, &c, 1, MSG_PEEK);
+            if (r == 0) {
+                aprset[i].rtnevents |= APR_POLLHUP;
+            }
+            else if (r < 0) {
+                aprset[i].rtnevents |= APR_POLLERR;
+            }
              aprset[i].rtnevents |= APR_POLLIN;
          }
          if (FD_ISSET(fd, &writeset)) {
@@ -399,6 +407,14 @@
              pollset->p->result_set[j] = pollset->p->query_set[i];
              pollset->p->result_set[j].rtnevents = 0;
              if (FD_ISSET(fd, &readset)) {
+                char c;
+                int r = recv(fd, &c, 1, MSG_PEEK);
+                if (r == 0) {
+                    pollset->p->result_set[j].rtnevents |= APR_POLLHUP;
+                }
+                else if (r < 0) {
+                    pollset->p->result_set[j].rtnevents |= APR_POLLERR;
+                }
                  pollset->p->result_set[j].rtnevents |= APR_POLLIN;
              }
              if (FD_ISSET(fd, &writeset)) {



-- 
-------------------------------------------
| Eric van der Maarel                     |
| NEDAP IDEAS                             |
| eric.vandermaarel@nedap.com             |
-------------------------------------------^[ZZ