You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by co...@apache.org on 2005/08/24 18:58:14 UTC

svn commit: r239711 - /httpd/httpd/trunk/server/mpm/prefork/prefork.c

Author: colm
Date: Wed Aug 24 09:58:11 2005
New Revision: 239711

URL: http://svn.apache.org/viewcvs?rev=239711&view=rev
Log:

Implement "de-listening" for graceful restarts with the prefork MPM. With this
change;

  1.) httpd -k graceful sends SIGUSR1 to the parent pid, which in turn
      sends SIGUSR1 to all of the active children,

  2.) Active children each close their copy of listener fd's.

This means that the listening sockets are freed for re-use. In the ordinary
case, this makes no difference. However if for example admin changes "Listen
80" to "Listen 81" in the config, this rev makes port 80 immediately available
(no waiting for the graceful children to die).


Modified:
    httpd/httpd/trunk/server/mpm/prefork/prefork.c

Modified: httpd/httpd/trunk/server/mpm/prefork/prefork.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/server/mpm/prefork/prefork.c?rev=239711&r1=239710&r2=239711&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/prefork/prefork.c (original)
+++ httpd/httpd/trunk/server/mpm/prefork/prefork.c Wed Aug 24 09:58:11 2005
@@ -331,6 +331,11 @@
     clean_child_exit(0);
 }
 
+static void stop_listening(int sig)
+{
+    ap_close_listeners();
+}
+
 /* volatile just in case */
 static int volatile shutdown_pending;
 static int volatile restart_pending;
@@ -715,10 +720,10 @@
          */
         apr_signal(SIGHUP, just_die);
         apr_signal(SIGTERM, just_die);
-        /* The child process doesn't do anything for AP_SIG_GRACEFUL.  
-         * Instead, the pod is used for signalling graceful restart.
+        /* The child process just closes listeners on AP_SIG_GRACEFUL.  
+         * The pod is used for signalling the graceful restart.
          */
-        apr_signal(AP_SIG_GRACEFUL, SIG_IGN);
+        apr_signal(AP_SIG_GRACEFUL, stop_listening);
         child_main(slot);
     }
 
@@ -1096,6 +1101,7 @@
 
     /* we've been told to restart */
     apr_signal(SIGHUP, SIG_IGN);
+    apr_signal(AP_SIG_GRACEFUL, SIG_IGN);
     if (one_process) {
         /* not worth thinking about */
         return 1;
@@ -1123,6 +1129,14 @@
         for (index = 0; index < ap_daemons_limit; ++index) {
             if (ap_scoreboard_image->servers[index][0].status != SERVER_DEAD) {
                 ap_scoreboard_image->servers[index][0].status = SERVER_GRACEFUL;
+                /* Ask each child to close its listeners.
+                 *
+                 * NOTE: we use the scoreboard, because if we send SIGUSR1 
+                 * to every process in the group, this may include CGI's, 
+                 * piped loggers, etc. They almost certainly won't handle 
+                 * it gracefully. 
+                 */
+                kill(ap_scoreboard_image->parent[index].pid, AP_SIG_GRACEFUL);
             }
         }
     }



Re: svn commit: r239711 - /httpd/httpd/trunk/server/mpm/prefork/prefork.c

Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Wed, Aug 24, 2005 at 08:21:35PM +0100, Joe Orton wrote:
> > It's not fixed *just* yet, only in prefork, and the PR reporter was
> > using worker. I had found the PR ;)
> 
> Duh, I missed that, sorry.

Oh I'm well ahead in the Duh stats ;) I've updated the CHANGES and PR
anyway, thanks for the stupidity filter :)

-- 
Colm MacCárthaigh                        Public Key: colm+pgp@stdlib.net

Re: svn commit: r239711 - /httpd/httpd/trunk/server/mpm/prefork/prefork.c

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Aug 24, 2005 at 08:18:35PM +0100, Colm MacCarthaigh wrote:
> On Wed, Aug 24, 2005 at 08:08:51PM +0100, Joe Orton wrote:
> > On Wed, Aug 24, 2005 at 12:02:37PM -0700, Justin Erenkrantz wrote:
> > > --On August 24, 2005 4:58:14 PM +0000 colm@apache.org wrote:
> > > >This means that the listening sockets are freed for re-use. In the
> > > >ordinary case, this makes no difference. However if for example admin
> > > >changes "Listen 80" to "Listen 81" in the config, this rev makes port 80
> > > >immediately available (no waiting for the graceful children to die).
> > > 
> > > Looks fine.  However, this is probably worth a CHANGES entry.
> > 
> > Especially since you win a free PR with this fix :)
> > 
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=28167
> 
> It's not fixed *just* yet, only in prefork, and the PR reporter was
> using worker. I had found the PR ;)

Duh, I missed that, sorry.

> I'm testing worker now. Is fixing it in the non-experimental MPM's
> enough to note it as fixed in the CHANGES file, or should I list the
> MPM's?

Having a CHANGES entry which explains the fix is limited to whatever set 
of MPMs sounds fine to me.

joe

Re: svn commit: r239711 - /httpd/httpd/trunk/server/mpm/prefork/prefork.c

Posted by Colm MacCarthaigh <co...@stdlib.net>.
On Wed, Aug 24, 2005 at 08:08:51PM +0100, Joe Orton wrote:
> On Wed, Aug 24, 2005 at 12:02:37PM -0700, Justin Erenkrantz wrote:
> > --On August 24, 2005 4:58:14 PM +0000 colm@apache.org wrote:
> > >This means that the listening sockets are freed for re-use. In the
> > >ordinary case, this makes no difference. However if for example admin
> > >changes "Listen 80" to "Listen 81" in the config, this rev makes port 80
> > >immediately available (no waiting for the graceful children to die).
> > 
> > Looks fine.  However, this is probably worth a CHANGES entry.
> 
> Especially since you win a free PR with this fix :)
> 
> http://issues.apache.org/bugzilla/show_bug.cgi?id=28167

It's not fixed *just* yet, only in prefork, and the PR reporter was
using worker. I had found the PR ;)

I'm testing worker now. Is fixing it in the non-experimental MPM's
enough to note it as fixed in the CHANGES file, or should I list the
MPM's?

-- 
Colm MacCárthaigh                        Public Key: colm+pgp@stdlib.net

Re: svn commit: r239711 - /httpd/httpd/trunk/server/mpm/prefork/prefork.c

Posted by Joe Orton <jo...@redhat.com>.
On Wed, Aug 24, 2005 at 12:02:37PM -0700, Justin Erenkrantz wrote:
> --On August 24, 2005 4:58:14 PM +0000 colm@apache.org wrote:
> >This means that the listening sockets are freed for re-use. In the
> >ordinary case, this makes no difference. However if for example admin
> >changes "Listen 80" to "Listen 81" in the config, this rev makes port 80
> >immediately available (no waiting for the graceful children to die).
> 
> Looks fine.  However, this is probably worth a CHANGES entry.

Especially since you win a free PR with this fix :)

http://issues.apache.org/bugzilla/show_bug.cgi?id=28167

joe

Re: svn commit: r239711 - /httpd/httpd/trunk/server/mpm/prefork/prefork.c

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On August 24, 2005 4:58:14 PM +0000 colm@apache.org wrote:

> Author: colm
> Date: Wed Aug 24 09:58:11 2005
> New Revision: 239711
>
> URL: http://svn.apache.org/viewcvs?rev=239711&view=rev
> Log:
>
> Implement "de-listening" for graceful restarts with the prefork MPM. With
> this change;
>
>   1.) httpd -k graceful sends SIGUSR1 to the parent pid, which in turn
>       sends SIGUSR1 to all of the active children,
>
>   2.) Active children each close their copy of listener fd's.
>
> This means that the listening sockets are freed for re-use. In the
> ordinary case, this makes no difference. However if for example admin
> changes "Listen 80" to "Listen 81" in the config, this rev makes port 80
> immediately available (no waiting for the graceful children to die).

Looks fine.  However, this is probably worth a CHANGES entry.

Thanks!  -- justin