Fix shutdown/restart hangs in the threaded MPM.

After removing mod_cgid from my build (thanks, Jeff), I can do:
* apachectl graceful, followed by
* apachectl restart, followed by
* apachectl stop

...and get the results you would expect.

Submitted by: Jeff Trawick, Greg Ames, Ryan Bloom

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@88941 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index baee74e04edf301a6153b3e724d4a18ccebc1710..6a8a4a62ca4a8b0e71e8d19dbfedc244dc5c4a21 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,5 +1,8 @@
 Changes with Apache 2.0.18-dev
 
+  *) Fix shutdown/restart hangs in the threaded MPM.
+     [Jeff Trawick, Greg Ames, Ryan Bloom]
+  
   *) Removed the keptalive boolean from conn_rec because it is now only
      used by a single routine and can be replaced by a local variable.
      [Greg Stein, Ryan Bloom, Roy Fielding]

diff --git a/server/mpm/threaded/threaded.c b/server/mpm/threaded/threaded.c
index f71301d303157d2e8ef215c94cc7ad47371d0d37..6a4bd53a6976ed0a12dc39f9c21dc022d0d2a711 100644 (file)
--- a/server/mpm/threaded/threaded.c
+++ b/server/mpm/threaded/threaded.c
@@ -705,12 +705,22 @@ static void child_main(int child_num_arg)
     
     /* What state should this child_main process be listed as in the scoreboard...?
      *  ap_update_child_status(my_child_num, i, SERVER_STARTING, (request_rec *) NULL);
+     * 
+     *  This state should be listed separately in the scoreboard, in some kind
+     *  of process_status, not mixed in with the worker threads' status.   
+     *  "life_status" is almost right, but it's in the worker's structure, and 
+     *  the name could be clearer.   gla
      */
 
     apr_signal_thread(check_signal);
 
+    workers_may_exit = 1;   /* helps us terminate a little more quickly when 
+                             * the dispatch of the signal thread
+                             * beats the Pipe of Death and the browsers
+                             */
+    
     /* A terminating signal was received. Now join each of the workers to clean them up.
-     *   If the worker already exitted, then the join frees their resources and returns.
+     *   If the worker already exited, then the join frees their resources and returns.
      *   If the worker hasn't exited, then this blocks until they have (then cleans up).
      */
     for (i = 0; i < ap_threads_per_child; i++) {
@@ -781,6 +791,29 @@ static int make_child(server_rec *s, int slot)
     return 0;
 }
 
+/* If there aren't many connections coming in from the network, the child 
+ * processes may need to be awakened from their network i/o waits.
+ * The pipe of death is an effective prod.
+ */
+   
+static void wake_up_and_die(void) 
+{
+    int i;
+    char char_of_death = '!';
+    apr_size_t one = 1;
+    apr_status_t rv;
+    
+    for (i = 0; i < ap_daemons_limit;) {
+        if ((rv = apr_file_write(pipe_of_death_out, &char_of_death, &one)) 
+                                 != APR_SUCCESS) {
+            if (APR_STATUS_IS_EINTR(rv)) continue;
+            ap_log_error(APLOG_MARK, APLOG_WARNING, rv, ap_server_conf, 
+                         "write pipe_of_death");
+        }
+        i++;
+    }
+}
+
 /* start up a bunch of children */
 static void startup_children(int number_to_start)
 {
@@ -994,7 +1027,6 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
 {
     int remaining_children_to_start;
     apr_status_t rv;
-    apr_size_t one = 1;
 
     pconf = _pconf;
     ap_server_conf = s;
@@ -1078,6 +1110,8 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
         /* Time to gracefully shut down:
          * Kill child processes, tell them to call child_exit, etc...
          */
+        wake_up_and_die();
+
         if (unixd_killpg(getpgrp(), SIGTERM) < 0) {
             ap_log_error(APLOG_MARK, APLOG_WARNING, errno, ap_server_conf, "killpg SIGTERM");
         }
@@ -1115,23 +1149,16 @@ int ap_mpm_run(apr_pool_t *_pconf, apr_pool_t *plog, server_rec *s)
     ++ap_my_generation;
     ap_scoreboard_image->global.running_generation = ap_my_generation;
     update_scoreboard_global();
-
+    
+    /* wake up the children...time to die.  But we'll have more soon */
+    wake_up_and_die();
+    
     if (is_graceful) {
-       int i, j;
-        char char_of_death = '!';
+        int i, j;
 
        ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_NOTICE, 0, ap_server_conf,
                    "SIGWINCH received.  Doing graceful restart");
 
-       /* give the children the signal to die */
-        for (i = 0; i < ap_daemons_limit;) {
-            if ((rv = apr_file_write(pipe_of_death_out, &char_of_death, &one)) != APR_SUCCESS) {
-                if (APR_STATUS_IS_EINTR(rv)) continue;
-                ap_log_error(APLOG_MARK, APLOG_WARNING, rv, ap_server_conf, "write pipe_of_death");
-            }
-            i++;
-        }
-
        /* This is mostly for debugging... so that we know what is still
          * gracefully dealing with existing request.
          */