Re: bug: squid hangs during https POST

Finished and available from the "ssl" branch
<http://devel.squid-cache.org/projects.html#ssl> together with other
pending SSL fixes.

[also attached, in isolated form]

The "poll()" version is verified to work. Have not verified the
"select()" version but it should work as well...

Regards
Henrik

Henrik Nordstrom wrote:
>
> The patch looks decent, but can be done better.
>
> It would be better if there was a flag set on the filedescriptor, than
> to call the pending_method all the time. This part of the code is
> accessed very frequently and is already a major bottleneck...
>
> Also, you still need to poll/select to keep a good balance in the
> processing power given to the filedescriptors. If not, a single SSL
> session could theoretically monopolize the usage. Skipping the
> poll/select only because there is some pending filedescriptor is no good
> as filedescriptors having data in their kernel buffers is as pending as
> the ones having data in their SSL buffers.. (only the notification
> capabilities differ).
>
> I'll work on a new patch. Should be available within an hour.
>
> Regards
> Henrik

Patch file generated Fri Oct 19 16:18:16 CEST 2001 from
CVS branch ssl
CVS base branch HEAD

Index: squid/src/comm_select.c
diff -u squid/src/comm_select.c:1.6 squid/src/comm_select.c:1.6.14.2
--- squid/src/comm_select.c:1.6 Fri May 4 06:39:12 2001
+++ squid/src/comm_select.c Fri Oct 19 07:13:10 2001
@@ -317,6 +317,7 @@
     int i;
     int maxfd;
     unsigned long nfds;
+ unsigned long npending;
     int num;
     int callicp = 0, callhttp = 0;
     int calldns = 0;
@@ -341,6 +342,7 @@
             comm_poll_http_incoming();
         callicp = calldns = callhttp = 0;
         nfds = 0;
+ npending = 0;
         maxfd = Biggest_FD + 1;
         for (i = 0; i < maxfd; i++) {
             int events;
@@ -370,19 +372,23 @@
                 pfds[nfds].events = events;
                 pfds[nfds].revents = 0;
                 nfds++;
+ if ((events & POLLRDNORM) && fd_table[i].flags.read_pending)
+ npending++;
             }
         }
         if (nfds == 0) {
             assert(shutting_down);
             return COMM_SHUTDOWN;
         }
+ if (npending)
+ msec = 0;
         if (msec > MAX_POLL_TIME)
             msec = MAX_POLL_TIME;
         for (;;) {
             statCounter.syscalls.polls++;
             num = poll(pfds, nfds, msec);
             statCounter.select_loops++;
- if (num >= 0)
+ if (num >= 0 || npending >= 0)
                 break;
             if (ignoreErrno(errno))
                 continue;
@@ -391,23 +397,28 @@
             return COMM_ERROR;
             /* NOTREACHED */
         }
- debug(5, num ? 5 : 8) ("comm_poll: %d FDs ready\n", num);
+ debug(5, num ? 5 : 8) ("comm_poll: %d+%d FDs ready\n", num, npending);
         statHistCount(&statCounter.select_fds_hist, num);
         /* Check timeout handlers ONCE each second. */
         if (squid_curtime > last_timeout) {
             last_timeout = squid_curtime;
             checkTimeouts();
         }
- if (num == 0)
+ if (num == 0 && npending == 0)
             continue;
         /* scan each socket but the accept socket. Poll this
          * more frequently to minimize losses due to the 5 connect
          * limit in SunOS */
         for (i = 0; i < nfds; i++) {
             fde *F;
- int revents;
- if (((revents = pfds[i].revents) == 0) || ((fd = pfds[i].fd) == -1))
+ int revents = pfds[i].revents;
+ fd = pfds[i].fd;
+ if (fd == -1)
                 continue;
+ if (fd_table[fd].flags.read_pending)
+ revents |= POLLIN;
+ if (revents == 0)
+ continue;
             if (fdIsIcp(fd)) {
                 callicp = 1;
                 continue;
@@ -632,6 +643,7 @@
 comm_select(int msec)
 {
     fd_set readfds;
+ fd_set pendingfds;
     fd_set writefds;
 #if DELAY_POOLS
     fd_set slowfds;
@@ -640,6 +652,7 @@
     int fd;
     int maxfd;
     int num;
+ int pending;
     int callicp = 0, callhttp = 0;
     int calldns = 0;
     int maxindex;
@@ -649,6 +662,7 @@
     int i;
 #endif
     fd_mask *fdsp;
+ fd_mask *pfdsp;
     fd_mask tmask;
     static time_t last_timeout = 0;
     struct timeval poll_time;
@@ -675,7 +689,8 @@
             howmany(maxfd, FD_MASK_BITS) * FD_MASK_BYTES);
         xmemcpy(&writefds, &global_writefds,
             howmany(maxfd, FD_MASK_BITS) * FD_MASK_BYTES);
- /* remove stalled FDs */
+ /* remove stalled FDs, and deal with pending descriptors */
+ pending = 0;
         maxindex = howmany(maxfd, FD_MASK_BITS);
         fdsp = (fd_mask *) & readfds;
         for (j = 0; j < maxindex; j++) {
@@ -700,6 +715,10 @@
                 default:
                     fatalf("bad return value from commDeferRead(FD %d)\n", fd);
                 }
+ if (FD_ISSET(fd, &readfds) && fd_table[fd].flags.read_pending) {
+ FD_SET(fd, &pendingfds);
+ pending++;
+ }
             }
         }
 #if DEBUG_FDBITS
@@ -727,13 +746,15 @@
         if (msec < 0)
             msec = MAX_POLL_TIME;
 #endif
+ if (pending)
+ msec = 0;
         for (;;) {
             poll_time.tv_sec = msec / 1000;
             poll_time.tv_usec = (msec % 1000) * 1000;
             statCounter.syscalls.selects++;
             num = select(maxfd, &readfds, &writefds, NULL, &poll_time);
             statCounter.select_loops++;
- if (num >= 0)
+ if (num >= 0 || pending > 0)
                 break;
             if (ignoreErrno(errno))
                 break;
@@ -743,10 +764,10 @@
             return COMM_ERROR;
             /* NOTREACHED */
         }
- if (num < 0)
+ if (num < 0 && !pending)
             continue;
- debug(5, num ? 5 : 8) ("comm_select: %d FDs ready at %d\n",
- num, (int) squid_curtime);
+ debug(5, num ? 5 : 8) ("comm_select: %d+%d FDs ready at %d\n",
+ num, pending, (int) squid_curtime);
         statHistCount(&statCounter.select_fds_hist, num);
         /* Check lifetime and timeout handlers ONCE each second.
          * Replaces brain-dead check every time through the loop! */
@@ -754,13 +775,14 @@
             last_timeout = squid_curtime;
             checkTimeouts();
         }
- if (num == 0)
+ if (num == 0 && pending == 0)
             continue;
         /* Scan return fd masks for ready descriptors */
         fdsp = (fd_mask *) & readfds;
+ pfdsp = (fd_mask *) & pendingfds;
         maxindex = howmany(maxfd, FD_MASK_BITS);
         for (j = 0; j < maxindex; j++) {
- if ((tmask = fdsp[j]) == 0)
+ if ((tmask = (fdsp[j] | pfdsp[j])) == 0)
                 continue; /* no bits here */
             for (k = 0; k < FD_MASK_BITS; k++) {
                 if (tmask == 0)
Index: squid/src/ssl_support.c
diff -u squid/src/ssl_support.c:1.4 squid/src/ssl_support.c:1.3.14.9
--- squid/src/ssl_support.c:1.4 Sun Aug 26 15:24:05 2001
+++ squid/src/ssl_support.c Fri Oct 19 07:13:10 2001
@@ -172,7 +270,17 @@
      char *buf;
      int len;
 {
- return (SSL_read(fd_table[fd].ssl, buf, len));
+ int i;
+
+ i = SSL_read(fd_table[fd].ssl, buf, len);
+
+ if (i > 0 && SSL_pending(fd_table[fd].ssl) > 0) {
+ debug(81, 2) ("SSL fd %d is pending\n", fd);
+ fd_table[fd].flags.read_pending = 1;
+ } else
+ fd_table[fd].flags.read_pending = 0;
+
+ return i;
 }
 
 int
Index: squid/src/structs.h
diff -u squid/src/structs.h:1.45 squid/src/structs.h:1.38.4.5
--- squid/src/structs.h:1.45 Thu Oct 18 13:52:11 2001
+++ squid/src/structs.h Fri Oct 19 05:52:08 2001
@@ -762,6 +763,7 @@
         unsigned int called_connect:1;
         unsigned int nodelay:1;
         unsigned int close_on_exec:1;
+ unsigned int read_pending:1;
     } flags;
     int bytes_read;
     int bytes_written;
Received on Fri Oct 19 2001 - 08:24:24 MDT