current jbsockets.c, 1.139, 1.140 jcc.c, 1.455, 1.456 loadcfg.c, 1.157, 1.158

Fabian Keil <[email protected]> Thu, 25 May 2017 11:16:58 +0000
Newsgroups gmane.comp.web.privoxy.cvs
Message-ID <[email protected]>
Update of /cvsroot/ijbswa/current
In directory sfp-cvs-1.v30.ch3.sourceforge.com:/tmp/cvs-serv1973

Modified Files:
	jbsockets.c jcc.c loadcfg.c 
Log Message:
Never use select() when poll() is available

On most platforms select() is limitted by FD_SETSIZE while
poll() is not. This was a scaling issue for multi-user setups.

Using poll() has no downside other than the usual risk
that code modifications may introduce new bugs that have
yet to be found and fixed.

At least in theory this commit could also reduce the latency
when there are lots of connections and select() would use
"bit fields in arrays of integers" to store file descriptors.

Another side effect is that Privoxy no longer has to stop
monitoring the client sockets when pipelined requests are
waiting but can't be read yet.

This code keeps the select()-based code behind ifdefs for
now but hopefully it can be removed soonish to make the
code more readable.

Sponsored by: Robert Klemme


Index: jcc.c
===================================================================
RCS file: /cvsroot/ijbswa/current/jcc.c,v
retrieving revision 1.455
retrieving revision 1.456
diff -C2 -d -r1.455 -r1.456
*** jcc.c	25 May 2017 11:16:04 -0000	1.455
--- jcc.c	25 May 2017 11:16:56 -0000	1.456
***************
*** 96,102 ****
--- 96,110 ----
  # endif
  
+ #ifdef HAVE_POLL
+ #ifdef __GLIBC__
+ #include <sys/poll.h>
+ #else
+ #include <poll.h>
+ #endif /* def __GLIBC__ */
+ #else
  # ifndef FD_ZERO
  #  include <select.h>
  # endif
+ #endif /* HAVE_POLL */
  
  #endif
***************
*** 1958,1964 ****
     char *hdr;
     char *p;
-    fd_set rfds;
     int n;
     jb_socket maxfd;
     int server_body;
     int ms_iis5_hack = 0;
--- 1966,1977 ----
     char *hdr;
     char *p;
     int n;
+ #ifdef HAVE_POLL
+    struct pollfd poll_fds[2];
+ #else
+    fd_set rfds;
     jb_socket maxfd;
+    struct timeval timeout;
+ #endif
     int server_body;
     int ms_iis5_hack = 0;
***************
*** 1970,1974 ****
     /* Skeleton for HTTP response, if we should intercept the request */
     struct http_response *rsp;
-    struct timeval timeout;
  #ifdef FEATURE_CONNECTION_KEEP_ALIVE
     int watch_client_socket;
--- 1983,1986 ----
***************
*** 1979,1984 ****
--- 1991,1998 ----
     http = csp->http;
  
+ #ifndef HAVE_POLL
     maxfd = (csp->cfd > csp->server_connection.sfd) ?
        csp->cfd : csp->server_connection.sfd;
+ #endif
  
     /* pass data between the client and server
***************
*** 1994,1997 ****
--- 2008,2012 ----
     for (;;)
     {
+ #ifndef HAVE_POLL
  #ifdef __OS2__
        /*
***************
*** 2015,2018 ****
--- 2030,2034 ----
  
        FD_SET(csp->server_connection.sfd, &rfds);
+ #endif /* ndef HAVE_POLL */
  
  #ifdef FEATURE_CONNECTION_KEEP_ALIVE
***************
*** 2059,2065 ****
--- 2075,2104 ----
  #endif  /* FEATURE_CONNECTION_KEEP_ALIVE */
  
+ #ifdef HAVE_POLL
+       poll_fds[0].fd = csp->cfd;
+ #ifdef FEATURE_CONNECTION_KEEP_ALIVE
+       if (!watch_client_socket)
+       {
+          /*
+           * Ignore incoming data, but still watch out
+           * for disconnects etc. These flags are always
+           * implied anyway but explicitly setting them
+           * doesn't hurt.
+           */
+          poll_fds[0].events = POLLERR|POLLHUP;
+       }
+       else
+ #endif
+       {
+          poll_fds[0].events = POLLIN;
+       }
+       poll_fds[1].fd = csp->server_connection.sfd;
+       poll_fds[1].events = POLLIN;
+       n = poll(poll_fds, 2, csp->config->socket_timeout * 1000);
+ #else
        timeout.tv_sec = csp->config->socket_timeout;
        timeout.tv_usec = 0;
        n = select((int)maxfd+1, &rfds, NULL, NULL, &timeout);
+ #endif /* def HAVE_POLL */
  
        if (n == 0)
***************
*** 2076,2080 ****
--- 2115,2123 ----
        else if (n < 0)
        {
+ #ifdef HAVE_POLL
+          log_error(LOG_LEVEL_ERROR, "poll() failed!: %E");
+ #else
           log_error(LOG_LEVEL_ERROR, "select() failed!: %E");
+ #endif
           mark_server_socket_tainted(csp);
           return;
***************
*** 2088,2092 ****
--- 2131,2149 ----
         * behind Privoxy's back.
         */
+ #ifdef HAVE_POLL
+       if ((poll_fds[0].revents & (POLLERR|POLLHUP|POLLNVAL)) != 0)
+       {
+          log_error(LOG_LEVEL_CONNECT,
+             "The client socket %d has become unusable while "
+             "the server socket %d is still open.",
+             csp->cfd, csp->server_connection.sfd);
+          mark_server_socket_tainted(csp);
+          break;
+       }
+ 
+       if (poll_fds[0].revents != 0)
+ #else
        if (FD_ISSET(csp->cfd, &rfds))
+ #endif /* def HAVE_POLL*/
        {
           int max_bytes_to_read = sizeof(buf) - 1;
***************
*** 2176,2180 ****
--- 2233,2241 ----
         * FIXME: Does `hdr' really mean `host'? No.
         */
+ #ifdef HAVE_POLL
+       if (poll_fds[1].revents != 0)
+ #else
        if (FD_ISSET(csp->server_connection.sfd, &rfds))
+ #endif /* HAVE_POLL */
        {
  #ifdef FEATURE_CONNECTION_KEEP_ALIVE
***************
*** 4049,4052 ****
--- 4110,4114 ----
     }
  
+ #ifndef HAVE_POLL
  #ifndef _WIN32
     if (bfd >= FD_SETSIZE)
***************
*** 4057,4060 ****
--- 4119,4123 ----
     }
  #endif
+ #endif
  
     if (haddr == NULL)

Index: jbsockets.c
===================================================================
RCS file: /cvsroot/ijbswa/current/jbsockets.c,v
retrieving revision 1.139
retrieving revision 1.140
diff -C2 -d -r1.139 -r1.140
*** jbsockets.c	24 Dec 2016 16:00:49 -0000	1.139
--- jbsockets.c	25 May 2017 11:16:56 -0000	1.140
***************
*** 212,217 ****
--- 212,221 ----
     int retval;
     jb_socket fd;
+ #ifdef HAVE_POLL
+    struct pollfd poll_fd[1];
+ #else
     fd_set wfds;
     struct timeval timeout;
+ #endif
  #if !defined(_WIN32) && !defined(__BEOS__) && !defined(AMIGA) && !defined(__OS2__)
     int   flags;
***************
*** 301,304 ****
--- 305,309 ----
        }
  
+ #ifndef HAVE_POLL
  #ifndef _WIN32
        if (fd >= FD_SETSIZE)
***************
*** 312,315 ****
--- 317,321 ----
        }
  #endif
+ #endif
  
  #ifdef FEATURE_EXTERNAL_FILTERS
***************
*** 364,367 ****
--- 370,379 ----
  #endif /* !defined(_WIN32) && !defined(__BEOS__) && !defined(AMIGA) && !defined(__OS2__) */
  
+ #ifdef HAVE_POLL
+    poll_fd[0].fd = fd;
+    poll_fd[0].events = POLLOUT;
+ 
+    if (poll(poll_fd, 1, 30000) > 0)
+ #else
        /* wait for connection to complete */
        FD_ZERO(&wfds);
***************
*** 374,377 ****
--- 386,390 ----
        if ((select((int)fd + 1, NULL, &wfds, NULL, &timeout) > 0)
           && FD_ISSET(fd, &wfds))
+ #endif
        {
           socklen_t optlen = sizeof(socket_error);
***************
*** 431,436 ****
--- 444,453 ----
     jb_socket fd;
     unsigned int addr;
+ #ifdef HAVE_POLL
+    struct pollfd poll_fd[1];
+ #else
     fd_set wfds;
     struct timeval tv[1];
+ #endif
  #if !defined(_WIN32) && !defined(__BEOS__) && !defined(AMIGA) && !defined(__OS2__)
     int   flags;
***************
*** 494,497 ****
--- 511,515 ----
     }
  
+ #ifndef HAVE_POLL
  #ifndef _WIN32
     if (fd >= FD_SETSIZE)
***************
*** 504,507 ****
--- 522,526 ----
     }
  #endif
+ #endif
  
     set_no_delay_flag(fd);
***************
*** 550,553 ****
--- 569,578 ----
  #endif /* !defined(_WIN32) && !defined(__BEOS__) && !defined(AMIGA) && !defined(__OS2__) */
  
+ #ifdef HAVE_POLL
+    poll_fd[0].fd = fd;
+    poll_fd[0].events = POLLOUT;
+ 
+    if (poll(poll_fd, 1, 30000) <= 0)
+ #else
     /* wait for connection to complete */
     FD_ZERO(&wfds);
***************
*** 559,562 ****
--- 584,588 ----
     /* MS Windows uses int, not SOCKET, for the 1st arg of select(). Weird! */
     if (select((int)fd + 1, NULL, &wfds, NULL, tv) <= 0)
+ #endif
     {
        close_socket(fd);
***************
*** 704,711 ****
  int data_is_available(jb_socket fd, int seconds_to_wait)
  {
     char buf[10];
     fd_set rfds;
     struct timeval timeout;
-    int n;
  
     memset(&timeout, 0, sizeof(timeout));
--- 730,745 ----
  int data_is_available(jb_socket fd, int seconds_to_wait)
  {
+    int n;
     char buf[10];
+ #ifdef HAVE_POLL
+    struct pollfd poll_fd[1];
+ 
+    poll_fd[0].fd = fd;
+    poll_fd[0].events = POLLIN;
+ 
+    n = poll(poll_fd, 1, seconds_to_wait * 1000);
+ #else
     fd_set rfds;
     struct timeval timeout;
  
     memset(&timeout, 0, sizeof(timeout));
***************
*** 721,724 ****
--- 755,759 ----
  
     n = select(fd+1, &rfds, NULL, NULL, &timeout);
+ #endif
  
     /*
***************
*** 1229,1233 ****
--- 1264,1273 ----
     int i;
     int max_selected_socket;
+ #ifdef HAVE_POLL
+    struct pollfd poll_fds[MAX_LISTENING_SOCKETS];
+    nfds_t polled_sockets;
+ #else
     fd_set selected_fds;
+ #endif
     jb_socket fd;
     const char *host_addr;
***************
*** 1236,1239 ****
--- 1276,1283 ----
     c_length = sizeof(client);
  
+ #ifdef HAVE_POLL
+    memset(poll_fds, 0, sizeof(poll_fds));
+    polled_sockets = 0;
+ #else
     /*
      * Wait for a connection on any socket.
***************
*** 1242,1245 ****
--- 1286,1290 ----
      */
     FD_ZERO(&selected_fds);
+ #endif
     max_selected_socket = 0;
     for (i = 0; i < MAX_LISTENING_SOCKETS; i++)
***************
*** 1247,1251 ****
--- 1292,1302 ----
        if (JB_INVALID_SOCKET != fds[i])
        {
+ #ifdef HAVE_POLL
+          poll_fds[i].fd = fds[i];
+          poll_fds[i].events = POLLIN;
+          polled_sockets++;
+ #else
           FD_SET(fds[i], &selected_fds);
+ #endif
           if (max_selected_socket < fds[i] + 1)
           {
***************
*** 1260,1264 ****
--- 1311,1319 ----
     do
     {
+ #ifdef HAVE_POLL
+       retval = poll(poll_fds, polled_sockets, -1);
+ #else
        retval = select(max_selected_socket, &selected_fds, NULL, NULL, NULL);
+ #endif
     } while (retval < 0 && errno == EINTR);
     if (retval <= 0)
***************
*** 1278,1283 ****
--- 1333,1342 ----
        return 0;
     }
+ #ifdef HAVE_POLL
+    for (i = 0; i < MAX_LISTENING_SOCKETS && (poll_fds[i].revents == 0); i++);
+ #else
     for (i = 0; i < MAX_LISTENING_SOCKETS && !FD_ISSET(fds[i], &selected_fds);
           i++);
+ #endif
     if (i >= MAX_LISTENING_SOCKETS)
     {
***************
*** 1326,1329 ****
--- 1385,1389 ----
  #endif
  
+ #ifndef HAVE_POLL
  #ifndef _WIN32
     if (afd >= FD_SETSIZE)
***************
*** 1336,1339 ****
--- 1396,1400 ----
     }
  #endif
+ #endif
  
  #ifdef FEATURE_EXTERNAL_FILTERS

Index: loadcfg.c
===================================================================
RCS file: /cvsroot/ijbswa/current/loadcfg.c,v
retrieving revision 1.157
retrieving revision 1.158
diff -C2 -d -r1.157 -r1.158
*** loadcfg.c	20 May 2017 09:24:35 -0000	1.157
--- loadcfg.c	25 May 2017 11:16:56 -0000	1.158
***************
*** 1376,1380 ****
              int max_client_connections = parse_numeric_value(cmd, arg);
  
! #ifndef _WIN32
              /*
               * Reject values below 1 for obvious reasons and values above
--- 1376,1380 ----
              int max_client_connections = parse_numeric_value(cmd, arg);
  
! #if !defined(_WIN32) && !defined(HAVE_POLL)
              /*
               * Reject values below 1 for obvious reasons and values above
***************
*** 1401,1404 ****
--- 1401,1407 ----
               * https://msdn.microsoft.com/en-us/library/windows/desktop/ms739169%28v=vs.85%29.aspx
               *
+              * On platforms were we use poll() we don't have to enforce
+              * an upper connection limit either.
+              *
               * XXX: Do OS/2, Amiga etc. belong here as well?
               */


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot