Merge r1606368 and r1607352 from trunk:

Follow up to r1527220/r1588852:

Implement better error checking/reporting around notification of abrupt parent
process termination.

It is likely that something bad is happening here based on these
user reports:

https://www.apachelounge.com/viewtopic.php?p=27848
http://mail-archives.apache.org/mod_mbox/httpd-users/201406.mbox/%3CCAC%2BRZnuwLD%2BJnoy2TYO8oeAWt6bFLMa%3DEhfDf9hS3cuuGUHXAw%40mail.gmail.com%3E

w-up to r1606368: HANDLE is PVOID which is void * (fix format string)

Submitted by: trawick
Reviewed by: covener, gsmith

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1608907 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index f81753b13664ec8f72718b6d82c9cef9975580bd..98c05c8b95b29f118f90b9c78b73aa41d4778d60 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,9 @@
 
 Changes with Apache 2.4.10
 
+  *) WinNT MPM: Improve error handling for termination events in child.
+     [Jeff Trawick]
+
   *) mod_proxy: When ping/pong is configured for a worker, don't send or
      forward "100 Continue" (interim) response to the client if it does
      not expect one. [Yann Ylavic]

diff --git a/STATUS b/STATUS
index be14904307c1f00f8b2bd081291b5e2b2fd2958d..91069627014db773c0189f016dde940c3494b1d0 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -100,15 +100,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-   * WinNT MPM diagnostics related to child checking for abrupt parent termination; I've
-     noticed 3 users so far with 2.4.9 reporting failures in this recently-modified
-     code.  (There are some references in one of the commits; I just found another
-     was on a German-language PHP list.)
-     trunk patch: http://svn.apache.org/r1606368
-                  http://svn.apache.org/r1607352
-     2.4.x patch: http://people.apache.org/~trawick/winnt_diagnostics_1606368_1607352.txt
-     +1: trawick, covener, gsmith
-               
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
@@ -194,15 +185,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
      2.4.x patch: http://people.apache.org/~jorton/ms_tmpdh-2.4.x.diff
      +1: jorton, 
 
-   * WinNT MPM diagnostics related to child checking for abrupt parent termination; I've
-     noticed 3 users so far with 2.4.9 reporting failures in this recently-modified
-     code.  (There are some references in one of the commits; I just found another
-     was on a German-language PHP list.)
-     trunk patch: http://svn.apache.org/r1606368
-                  http://svn.apache.org/r1607352
-     2.4.x patch: http://people.apache.org/~trawick/winnt_diagnostics_1606368_1607352.txt
-     +1: trawick, covener, gsmith
-               
 OTHER PROPOSALS
 
    * A list of further possible backports can be found at: 

diff --git a/server/mpm/winnt/child.c b/server/mpm/winnt/child.c
index 6879179fa1e404399d3ae8699fa26b9423a56f83..4ccf64894d2ad81e81d8411c12dd8a7b20e4159e 100644 (file)
--- a/server/mpm/winnt/child.c
+++ b/server/mpm/winnt/child.c
@@ -986,7 +986,15 @@ void child_main(apr_pool_t *pconf, DWORD parent_pid)
 
     if (parent_pid != my_pid) {
         child_events[2] = OpenProcess(SYNCHRONIZE, FALSE, parent_pid);
-        num_events = 3;
+        if (child_events[2] == NULL) {
+            num_events = 2;
+            ap_log_error(APLOG_MARK, APLOG_ERR, apr_get_os_error(), ap_server_conf, APLOGNO(02643)
+                         "Child: Failed to open handle to parent process %ld; "
+                         "will not react to abrupt parent termination", parent_pid);
+        }
+        else {
+            num_events = 3;
+        }
     }
     else {
         /* presumably -DONE_PROCESS */
@@ -1084,7 +1092,7 @@ void child_main(apr_pool_t *pconf, DWORD parent_pid)
         apr_sleep(1 * APR_USEC_PER_SEC);
     }
 
-    /* Wait for one of three events:
+    /* Wait for one of these events:
      * exit_event:
      *    The exit_event is signaled by the parent process to notify
      *    the child that it is time to exit.
@@ -1093,6 +1101,8 @@ void child_main(apr_pool_t *pconf, DWORD parent_pid)
      *    This event is signaled by the worker threads to indicate that
      *    the process has handled MaxConnectionsPerChild connections.
      *
+     * parent process exiting
+     *
      * TIMEOUT:
      *    To do periodic maintenance on the server (check for thread exits,
      *    number of completion contexts, etc.)
@@ -1113,6 +1123,7 @@ void child_main(apr_pool_t *pconf, DWORD parent_pid)
         rv = WaitForMultipleObjects(num_events, (HANDLE *)child_events, FALSE, INFINITE);
         cld = rv - WAIT_OBJECT_0;
 #else
+        /* THIS IS THE EXPECTED BUILD VARIATION */
         rv = WaitForMultipleObjects(num_events, (HANDLE *)child_events, FALSE, 1000);
         cld = rv - WAIT_OBJECT_0;
         if (rv == WAIT_TIMEOUT) {
@@ -1125,6 +1136,17 @@ void child_main(apr_pool_t *pconf, DWORD parent_pid)
             ap_log_error(APLOG_MARK, APLOG_CRIT, apr_get_os_error(),
                          ap_server_conf, APLOGNO(00356)
                          "Child: WAIT_FAILED -- shutting down server");
+            /* check handle validity to identify a possible culprit */
+            for (i = 0; i < num_events; i++) {
+                DWORD out_flags;
+
+                if (0 == GetHandleInformation(child_events[i], &out_flags)) {
+                    ap_log_error(APLOG_MARK, APLOG_CRIT, apr_get_os_error(),
+                                 ap_server_conf, APLOGNO(02644)
+                                 "Child: Event handle #%d (%pp) is invalid",
+                                 i, child_events[i]);
+                }
+            }
             break;
         }
         else if (cld == 0) {