httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@attglobal.net>
Subject [PATCH] Re: 2.0.26 tag?
Date Mon, 15 Oct 2001 19:24:02 GMT
Greg Ames <gregames@remulak.net> writes:

> Justin Erenkrantz wrote:
> 
> > Any major complaints or showstoppers that can be addressed in a short
> > timeframe (i.e. a week)?  -- justin
> 
> I'm seeing a loop in child_main in prefork issuing selects() if I run
> the server as me (not root) with 
> 
> Listen 80
> Listen 8092
> 
> in my config.  It works fine as root.  
> 
> I suspect this has something to do with the change that keeps
> ap_listen_rec's with active == 0 around, but haven't found the bug yet. 
> We may want to just keep a clean version of the ap_listeners list on
> general principles, and hang the inactive guys somewhere else.

Here is a patch to keep inactive ap_listen_recs out of the
ap_listeners list, but

1) I didn't implement a list of broken recs to keep the storage around
   (useful, but bigger problems to solve right now)

2) there is some bogosity going on w.r.t. file descriptor 2, most
   likely related to its use as stderr

   In testing this patch, I had "listen 80" and "listen 8080", started
   as non-root.  When I send the parent SIGHUP and the config is
   processed again, file descriptor 2 is not in use at the time
   socket() is called so I get file descriptor 2 for the socket
   associated with the ap_listen_rec for "listen 80."  By the time
   make_sock() is called, somebody has rightfully changed 2 to point
   to the error log.

   2 should be /dev/null or otherwise be open during config processing
   to prevent files opened during config processing from getting
   invalid descriptors.

Anyway...

Index: server/listen.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/listen.c,v
retrieving revision 1.62
diff -u -r1.62 listen.c
--- server/listen.c	2001/10/04 20:00:53	1.62
+++ server/listen.c	2001/10/15 19:04:40
@@ -270,7 +270,7 @@
 {
     apr_pool_t *pconf = process->pconf;
     ap_listen_rec *lr;
-    ap_listen_rec *next;
+    ap_listen_rec *next, *prev;
     int num_open;
 
     /* Don't allocate a default listener.  If we need to listen to a
@@ -278,7 +278,7 @@
      * config file.
      */
     num_open = 0;
-    for (lr = ap_listeners; lr; lr = lr->next) {
+    for (lr = ap_listeners, prev = NULL; lr; prev = lr, lr = lr->next) {
 	if (lr->active) {
 	    ++num_open;
 	}
@@ -287,6 +287,26 @@
 		++num_open;
 		lr->active = 1;
 	    }
+            else {
+                /* prune this ap_listen_rec out of the list; it is broken
+                 * now, since make_sock() failed and the socket is closed;
+                 * also, it sucks to have inactive sockets in the 
+                 * ap_listeners list since that causes problems for the
+                 * MPMs
+                 */
+                if (prev) {
+                    prev->next = lr->next;
+                }
+                else {
+                    AP_DEBUG_ASSERT(lr == ap_listeners);
+                    ap_listeners = ap_listeners->next;
+                }
+                /* XXX if you wanna avoid a leak across restarts of broken 
+                 * ap_listen_recs, add lr to a list we'll consult next time
+                 * we process the config; note that the socket in lr has
+                 * been closed
+                 */
+            }
 	}
     }
 
Index: server/mpm/prefork/prefork.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/mpm/prefork/prefork.c,v
retrieving revision 1.203
diff -u -r1.203 prefork.c
--- server/mpm/prefork/prefork.c	2001/10/11 16:28:23	1.203
+++ server/mpm/prefork/prefork.c	2001/10/15 19:04:52
@@ -663,14 +663,12 @@
 		}
 		first_lr=lr;
 		do {
-                    if (lr->active) {
-                        apr_os_sock_get(&sockdes, lr->sd);
-		        if (FD_ISSET(sockdes, &main_fds))
-			    goto got_listener;
-                        lr = lr->next;
-                        if (!lr)
-                            lr = ap_listeners;
-                    }
+                    apr_os_sock_get(&sockdes, lr->sd);
+                    if (FD_ISSET(sockdes, &main_fds))
+                        goto got_listener;
+                    lr = lr->next;
+                    if (!lr)
+                        lr = ap_listeners;
 		}
 		while (lr != first_lr);
 		/* FIXME: if we get here, something bad has happened, and we're
@@ -1090,12 +1088,10 @@
     listenmaxfd = -1;
     FD_ZERO(&listenfds);
     for (lr = ap_listeners; lr; lr = lr->next) {
-        if (lr->active) {
-            apr_os_sock_get(&sockdes, lr->sd);
-            FD_SET(sockdes, &listenfds);
-            if (sockdes > listenmaxfd) {
-                listenmaxfd = sockdes;
-            }
+        apr_os_sock_get(&sockdes, lr->sd);
+        FD_SET(sockdes, &listenfds);
+        if (sockdes > listenmaxfd) {
+            listenmaxfd = sockdes;
         }
     }
     return 0;


-- 
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...

Mime
View raw message