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.vandermaa...@nedap.com             |
-------------------------------------------^[ZZ

Reply via email to