[tbnl-devel] Socket leaks, again

[Edi Weitz]

[Again, I'm in a train and sending via cell phone and GPRS. Let's see
if it works better this time. Also, I'm not sure if SquirrelMail will
be able to keep indentations.]

I'm prepared to make myself a fool in public again. I've debugged the
mod_lisp/Apache interaction (by inserting log statements in strategic
places) a bit and here's what I found out (Linux x86, mod_lisp 2.33,
Apache 1.3.29, CMUCL 18e):

1. Whenever OpenLispSocket is called, cfg->LispSocket is 0. This can
   be best seen if you start Apache with the -X option so that you
   only one child process. This would imply that sockets can never be
   re-used.

2. Another data point (see also Peter's earlier post) is that

     netstat -an | grep 3000

   reports far more established connections than there are Apache
   child processes.

3. I guess this is due to the fact that the configuration record is
   reset by Apache on each request. I observe that the comment in
   Apache's example module (which is also in mod_lisp) says

     "Note that while the per-directory and per-server configuration
      records are available to most of the module handlers, they
      should be treated as READ-ONLY by all except the command and
      merge handlers.  Sometimes handlers are handed a record that
      applies to the current location by implication or inheritance,
      and modifying it will change the rules for other locations."

   This is obviously violated by mod_lisp which modifies the record in
   the content handler.

4. My naïve attempt to fix this is to make LispSocket and
   UnsafeLispSocket global variables. A patch which reflects my
   changes is attached at the bottom. This currently works for
   me. Debugging shows that sockets are in fact re-used this
   time. Also, if I benchmark with ApacheBench (2000 request, 200
   concurrent, 1300 byte static page, client and server on same
   machine) then the original mod_lisp code is able to handle roughly
   140 requests per seconds while my modified code handles more than
   460 requests per seconds.

5. Of course, this patch will only work if the Apache children are
   separate processes spawned by the server process as with Apache
   1.3, i.e. another approach must be taken for the new, alternative
   Apache 2.0 thread implementations.

OK, that's all for now. Let me know what you think.

Cheers,
Edi.


--- /home/edi/downloads/Lisp/mod_lisp-2.33.c	2004-07-16 01:52:23.000000000
+0200
+++ mod_lisp-2.33.c	2004-07-18 16:58:23.000000000 +0200
@@ -189,10 +189,11 @@
   char LispServerIP[20];
   long LispServerPort;
   char LispServerId[100];
-  long LispSocket;
-  long UnsafeLispSocket;
 } excfg;

+static long LispSocket = 0;
+static long UnsafeLispSocket = 0;
+
 pool *SocketPool = NULL;

 /*
@@ -371,13 +372,6 @@
     if (lisp_pool == NULL) {
         lisp_pool = ap_make_sub_pool(NULL);
     };
-    /*
-     * Likewise for the table of routine/environment pairs we visit
outside of
-     * request context.
-     */
-    if (static_calls_made == NULL) {
-        static_calls_made = ap_make_table(lisp_pool, 16);
-    };
 }
 #endif

@@ -421,21 +415,21 @@
   int ret;

 #ifndef WIN32
-  if (cfg->LispSocket)
-    if (cfg->UnsafeLispSocket)
+  if (LispSocket)
+    if (UnsafeLispSocket)
       {
-	ap_pclosesocket(SocketPool, cfg->LispSocket);
-	cfg->LispSocket = 0;
-	cfg->UnsafeLispSocket = 0;
+	ap_pclosesocket(SocketPool, LispSocket);
+	LispSocket = 0;
+	UnsafeLispSocket = 0;
       }
     else
       {
-	return cfg->LispSocket;
+	return LispSocket;
       }
 #endif

-  cfg->LispSocket = 0;
-  cfg->UnsafeLispSocket = 0;
+  LispSocket = 0;
+  UnsafeLispSocket = 0;
   addr.sin_addr.s_addr = inet_addr(cfg->LispServerIP);
   addr.sin_port = htons((unsigned short) cfg->LispServerPort);
   addr.sin_family = AF_INET;
@@ -459,7 +453,7 @@
   if (ret == -1)
     return -1;

-  cfg->LispSocket = sock;
+  LispSocket = sock;

   return sock;
 }
@@ -498,13 +492,13 @@
   if (Socket != -1)
     ap_pclosesocket(SocketPool, Socket);
 #else
-  if (!cfg->LispSocket)
+  if (!LispSocket)
     return;

-  ap_pclosesocket(SocketPool, cfg->LispSocket);
+  ap_pclosesocket(SocketPool, LispSocket);

-  cfg->LispSocket = 0;
-  cfg->UnsafeLispSocket = 0;
+  LispSocket = 0;
+  UnsafeLispSocket = 0;
 #endif
 }

@@ -605,7 +599,7 @@
     }
   ap_reset_timeout(r);

-  dcfg->UnsafeLispSocket = 1;
+  UnsafeLispSocket = 1;

   if (r->subprocess_env)
     {
@@ -860,7 +854,7 @@
       ap_bpushfd(BuffSocket, -1, -1); /* unlink buffer to keep socket */
       BuffSocket->flags &= ~B_SOCKET;
     }
-  dcfg->UnsafeLispSocket = 0;
+  UnsafeLispSocket = 0;
   return OK;
 }

@@ -985,7 +979,6 @@
     strcpy(cfg->LispServerId, "apache");
     cfg->LispServerPort = 3000;
     cfg->DefaultLispServer = 1;
-    cfg->LispSocket = 0;

     cfg->cmode = CONFIG_MODE_DIRECTORY;
     dname = (dname != NULL) ? dname : "";
@@ -1059,7 +1052,6 @@
 	  merged_config->DefaultLispServer = 1;
 	}

-    merged_config->LispSocket = 0;
     return (void *) merged_config;
 }

@@ -1087,7 +1079,6 @@
     strcpy(cfg->LispServerIP, "127.0.0.1");
     strcpy(cfg->LispServerId, "apache");
     cfg->LispServerPort = 3000;
-    cfg->LispSocket = 0;
     cfg->DefaultLispServer = 1;
     sname = (sname != NULL) ? sname : "";
     cfg->loc = ap_pstrcat(p, "SVR(", sname, ")", NULL);
@@ -1148,7 +1139,6 @@
 	  merged_config->DefaultLispServer = 1;
 	}

-    merged_config->LispSocket = 0;
     return (void *) merged_config;
 }

@@ -1309,7 +1299,6 @@
   cfg->LispServerId[99] = 0;
   cfg->LispServerPort = atoi(port);
   cfg->DefaultLispServer = 0;
-  cfg->LispSocket = 0;

   return NULL;
 }