---------------------
PatchSet 3863 
Date: 2006/10/19 06:12:56
Author: rousskov
Branch: squid3-icap
Tag: (none) 
Log:
- Make sure there is an accept callback when we are accepting.
  If there is no callback and we accept, we will silently leak the
  accepted FD.  When we are running out of FDs, there is often no
  accept callback.

  The old code, when running out of FDs, would create lots of
  "orphaned" or "forgotten" FDs that will eventually get into a
  CLOSED_WAIT state and remain there until Squid quits.

  The new code does not call accept() if there is no accept
  callback and does not register the accept FD for reading if the
  AcceptLimiter is deferring, because when the AcceptLimiter
  kicks in, it will register the accept FD for reading. There are
  most likely other places/cases where accept FD should not be
  registered for reading.

Members: 
	src/comm.cc:1.53.2.4->1.53.2.5 
	src/comm.h:1.18.4.1->1.18.4.2 

Index: squid3/src/comm.cc
===================================================================
RCS file: /cvsroot/squid-sf//squid3/src/comm.cc,v
retrieving revision 1.53.2.4
retrieving revision 1.53.2.5
diff -u -r1.53.2.4 -r1.53.2.5
--- squid3/src/comm.cc	29 Sep 2006 23:27:11 -0000	1.53.2.4
+++ squid3/src/comm.cc	19 Oct 2006 06:12:56 -0000	1.53.2.5
@@ -1,6 +1,6 @@
 
 /*
- * $Id: comm.cc,v 1.53.2.4 2006/09/29 23:27:11 dwsquid Exp $
+ * $Id: comm.cc,v 1.53.2.5 2006/10/19 06:12:56 rousskov Exp $
  *
  * DEBUG: section 5     Socket Functions
  * AUTHOR: Harvest Derived
@@ -2063,6 +2063,18 @@
 
 void
 fdc_t::acceptOne(int fd) {
+    // If there is no callback and we accept, we will leak the accepted FD.
+    // When we are running out of FDs, there is often no callback.
+    if (!accept.accept.callback.handler) {
+        debug (5,5) ("fdc_t::acceptOne orphaned: FD %d\n", fd);
+        // XXX: can we remove this and similar "just in case" calls and 
+        // either listen always or listen only when there is a callback?
+        if (!AcceptLimiter::Instance().deferring())
+            commSetSelect(fd, COMM_SELECT_READ, comm_accept_try, NULL, 0);
+        accept.accept.finished(true);
+        return;
+    }
+
     /*
      * We don't worry about running low on FDs here.  Instead,
      * httpAccept() will use AcceptLimiter if we reach the limit
@@ -2077,6 +2089,7 @@
     if (newfd < 0) {
         if (newfd == COMM_NOMESSAGE) {
             /* register interest again */
+            debug (5,5) ("fdc_t::acceptOne eof: FD %d handler: %p\n", fd, (void*)accept.accept.callback.handler);
             commSetSelect(fd, COMM_SELECT_READ, comm_accept_try, NULL, 0);
             accept.accept.finished(true);
             return;
@@ -2092,6 +2105,9 @@
         return;
     }
 
+    debug (5,5) ("fdc_t::acceptOne accepted: FD %d handler: %p newfd: %d\n", fd, (void*)accept.accept.callback.handler, newfd);
+
+    assert(accept.accept.callback.handler);
     accept.accept.doCallback(fd, newfd, COMM_OK, 0, &accept.connDetails);
 
     /* If we weren't re-registed, don't bother trying again! */
@@ -2136,6 +2152,7 @@
  */
 void
 comm_accept(int fd, IOACB *handler, void *handler_data) {
+    debug (5,5) ("comm_accept: FD %d handler: %p\n", fd, (void*)handler);
     requireOpenAndActive(fd);
 
     /* make sure we're not pending! */
@@ -2209,8 +2226,14 @@
     return Instance_;
 }
 
+bool
+AcceptLimiter::deferring() const {
+    return deferred.size() > 0;
+}
+
 void
 AcceptLimiter::defer (int fd, Acceptor::AcceptorFunction *aFunc, void *data) {
+    debug (5,5) ("AcceptLimiter::defer: FD %d handler: %p\n", fd, (void*)aFunc);
     Acceptor temp;
     temp.theFunction = aFunc;
     temp.acceptFD = fd;
@@ -2220,7 +2243,7 @@
 
 void
 AcceptLimiter::kick() {
-    if (!deferred.size())
+    if (!deferring())
         return;
 
     /* Yes, this means the first on is the last off....
Index: squid3/src/comm.h
===================================================================
RCS file: /cvsroot/squid-sf//squid3/src/comm.h,v
retrieving revision 1.18.4.1
retrieving revision 1.18.4.2
diff -u -r1.18.4.1 -r1.18.4.2
--- squid3/src/comm.h	29 Sep 2006 23:27:11 -0000	1.18.4.1
+++ squid3/src/comm.h	19 Oct 2006 06:12:57 -0000	1.18.4.2
@@ -114,6 +114,8 @@
     void defer (int, Acceptor::AcceptorFunction *, void *);
     void kick();
 
+    bool deferring() const;
+
 private:
     static AcceptLimiter Instance_;
     Vector<Acceptor> deferred;