You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rb...@apache.org on 2001/08/13 06:25:43 UTC

cvs commit: httpd-2.0/server listen.c

rbb         01/08/12 21:25:43

  Modified:    .        CHANGES
               server   listen.c
  Log:
  Close a major resource leak.  Everytime we had issued a
  graceful restart, we leaked a socket descriptor.
  
  The listening sockets should not be set inheritable, at least
  not at this point.  We only want some of the httpd children to
  inherit the socket.  Namely, those that will be actually serving
  requests.  Any other child process (piped logs), should not be
  inheriting the sockets.
  
  PR:	7891
  
  Revision  Changes    Path
  1.289     +4 -0      httpd-2.0/CHANGES
  
  Index: CHANGES
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/CHANGES,v
  retrieving revision 1.288
  retrieving revision 1.289
  diff -u -r1.288 -r1.289
  --- CHANGES	2001/08/11 04:09:45	1.288
  +++ CHANGES	2001/08/13 04:25:42	1.289
  @@ -1,5 +1,9 @@
   Changes with Apache 2.0.24-dev
   
  +  *) Close a major resource leak.  Everytime we had issued a
  +     graceful restart, we leaked a socket descriptor.
  +     [Ryan Bloom]
  +
     *) Fix a problem with the new method code.  We need to cast
        the 1 to an apr_int64_t or it will be treated as a 32-bit
        integer, and it will wrap after being shifted 32 times.
  
  
  
  1.60      +0 -1      httpd-2.0/server/listen.c
  
  Index: listen.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/server/listen.c,v
  retrieving revision 1.59
  retrieving revision 1.60
  diff -u -r1.59 -r1.60
  --- listen.c	2001/07/26 16:36:40	1.59
  +++ listen.c	2001/08/13 04:25:43	1.60
  @@ -259,7 +259,6 @@
                        "alloc_listener: failed to get a socket for %s", addr);
           return;
       }
  -    apr_socket_set_inherit(new->sd);
       new->next = ap_listeners;
       ap_listeners = new;
   }
  
  
  

Re: cvs commit: httpd-2.0/server listen.c

Posted by Bill Stoddard <bi...@wstoddard.com>.
> On Sunday 12 August 2001 21:25, rbb@apache.org wrote:
> > rbb         01/08/12 21:25:43
> >
> >   Modified:    .        CHANGES
> >                server   listen.c
> >   Log:
> >   Close a major resource leak.  Everytime we had issued a
> >   graceful restart, we leaked a socket descriptor.
> >
> >   The listening sockets should not be set inheritable, at least
> >   not at this point.  We only want some of the httpd children to
> >   inherit the socket.  Namely, those that will be actually serving
> >   requests.  Any other child process (piped logs), should not be
> >   inheriting the sockets.
>
> This is a pretty major resource leak.  We have had this leak for a very
> long time, and I am pretty sure that this is a big enough fix (especially
> with the APR pools patch), that it should be in the next release.
>
> I am not giving 2.0.23 a -1 for release, but I would like to roll 2.0.24
> next week.

2.0.23 is dead. Greg cannot keep it running on apache.org because of a segfault in
mod_mime.c

Bill


Re: cvs commit: httpd-2.0/server listen.c

Posted by Ryan Bloom <rb...@covalent.net>.
On Sunday 12 August 2001 21:25, rbb@apache.org wrote:
> rbb         01/08/12 21:25:43
>
>   Modified:    .        CHANGES
>                server   listen.c
>   Log:
>   Close a major resource leak.  Everytime we had issued a
>   graceful restart, we leaked a socket descriptor.
>
>   The listening sockets should not be set inheritable, at least
>   not at this point.  We only want some of the httpd children to
>   inherit the socket.  Namely, those that will be actually serving
>   requests.  Any other child process (piped logs), should not be
>   inheriting the sockets.

This is a pretty major resource leak.  We have had this leak for a very
long time, and I am pretty sure that this is a big enough fix (especially
with the APR pools patch), that it should be in the next release.

I am not giving 2.0.23 a -1 for release, but I would like to roll 2.0.24
next week.  I am pretty sure that this patch breaks Windows and
potentially OS/2 and BeOS though.

The problem is how the inherit flag works.  On Unix, we have two 
ways to create child processes, apr_fork and apr_proc_create.  apr_fork
doesn't recognize the inherit flag by design.  apr_proc_create does.
So, by not setting this to be inheritable, we get it in the child servers,
but not the piped loggers.  I have no idea how to fix this on other platforms,
so I leave that to people who have access to those platforms.

One option would be to set the socket as non-inheritable, and then before
creating the child servers, re-set it to inheritable.  This would require
walking the listener list however.

The only other big bug that we definately have to fix, is the CGI bug that
Doug posted earlier today.  Once that is fixed, I will roll 2.0.24.

Ryan
______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------

Re: cvs commit: httpd-2.0/server listen.c

Posted by Ryan Bloom <rb...@covalent.net>.
On Sunday 12 August 2001 21:25, rbb@apache.org wrote:
> rbb         01/08/12 21:25:43
>
>   Modified:    .        CHANGES
>                server   listen.c
>   Log:
>   Close a major resource leak.  Everytime we had issued a
>   graceful restart, we leaked a socket descriptor.
>
>   The listening sockets should not be set inheritable, at least
>   not at this point.  We only want some of the httpd children to
>   inherit the socket.  Namely, those that will be actually serving
>   requests.  Any other child process (piped logs), should not be
>   inheriting the sockets.

This is a pretty major resource leak.  We have had this leak for a very
long time, and I am pretty sure that this is a big enough fix (especially
with the APR pools patch), that it should be in the next release.

I am not giving 2.0.23 a -1 for release, but I would like to roll 2.0.24
next week.  I am pretty sure that this patch breaks Windows and
potentially OS/2 and BeOS though.

The problem is how the inherit flag works.  On Unix, we have two 
ways to create child processes, apr_fork and apr_proc_create.  apr_fork
doesn't recognize the inherit flag by design.  apr_proc_create does.
So, by not setting this to be inheritable, we get it in the child servers,
but not the piped loggers.  I have no idea how to fix this on other platforms,
so I leave that to people who have access to those platforms.

One option would be to set the socket as non-inheritable, and then before
creating the child servers, re-set it to inheritable.  This would require
walking the listener list however.

The only other big bug that we definately have to fix, is the CGI bug that
Doug posted earlier today.  Once that is fixed, I will roll 2.0.24.

Ryan
______________________________________________________________
Ryan Bloom                        	rbb@apache.org
Covalent Technologies			rbb@covalent.net
--------------------------------------------------------------