mpm_winnt: Prevent the parent-child pipe from leaking into other

spawned processes, and ensure we have a /Device/null handle for
stdout when running as-a-service.  [William Rowe]

Backport: r568446

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

diff --git a/CHANGES b/CHANGES
index 58a17c11884fcc23b1a2f32f2a2b745743e9c022..b98127da05650bd21bcf6b63adcc95ab27080327 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.2.6
 
+  *) mpm_winnt: Prevent the parent-child pipe from leaking into other
+     spawned processes, and ensure we have a /Device/null handle for
+     stdout when running as-a-service.  [William Rowe]
+
   *) mod_ldap: Avoid possible crashes, hangs, and busy loops due to
      improper merging of the cache lock in vhost config
      PR 43164 [Eric Covener]

diff --git a/STATUS b/STATUS
index 914c31e21161eb7b2d8038dd361f5af2daeb66f4..7bfa76c8edf999aa9c13fb1c3b8f4007a55ca2c0 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -290,12 +290,6 @@ PATCHES PROPOSED TO BACKPORT FROM TRUNK:
       http://svn.apache.org/viewvc?view=rev&revision=565671
       +1: niq
  
-    * mpm_winnt: Prevent the parent-child pipe from leaking into other
-      spawned processes, and ensure we have a /Device/null handle for
-      stdout when running as-a-service.  [William Rowe]
-        http://svn.apache.org/viewvc?view=rev&revision=568446
-        +1: wrowe
-
     * log core: fix the new piped logger case where we couldn't connect 
       the replacement stderr logger's stderr to the NULL stdout stream.  
       Continue in this case, since the previous alternative of no error 

diff --git a/server/mpm/winnt/mpm_winnt.c b/server/mpm/winnt/mpm_winnt.c
index e55cbb4651ddaf72a53249dd93fea9a0059aeb30..4b37d7414adef2b735b8ed2ad738331da8e8e517 100644 (file)
--- a/server/mpm/winnt/mpm_winnt.c
+++ b/server/mpm/winnt/mpm_winnt.c
@@ -66,7 +66,6 @@ static int thread_limit = DEFAULT_THREAD_LIMIT;
 static int first_thread_limit = 0;
 static int changed_limit_at_restart;
 int winnt_mpm_state = AP_MPMQ_STARTING;
-
 /* ap_my_generation are used by the scoreboard code */
 ap_generation_t volatile ap_my_generation=0;
 
@@ -87,6 +86,12 @@ void child_main(apr_pool_t *pconf);
 extern apr_proc_mutex_t *start_mutex;
 extern HANDLE exit_event;
 
+/* Only one of these, the pipe from our parent, ment only for
+ * one child worker's consumption (not to be inherited!)
+ * XXX: decorate this name for the trunk branch, was left simplified
+ *      only to make the 2.2 patch trivial to read.
+ */
+static HANDLE pipe;
 
 /* Stub functions until this MPM supports the connection status API */
 
@@ -330,7 +335,6 @@ void get_handles_from_parent(server_rec *s, HANDLE *child_exit_event,
                              apr_proc_mutex_t **child_start_mutex,
                              apr_shm_t **scoreboard_shm)
 {
-    HANDLE pipe;
     HANDLE hScore;
     HANDLE ready_event;
     HANDLE os_start;
@@ -338,7 +342,9 @@ void get_handles_from_parent(server_rec *s, HANDLE *child_exit_event,
     void *sb_shared;
     apr_status_t rv;
 
-    pipe = GetStdHandle(STD_INPUT_HANDLE);
+    /* *** We now do this was back in winnt_rewrite_args
+     * pipe = GetStdHandle(STD_INPUT_HANDLE);
+     */
     if (!ReadFile(pipe, &ready_event, sizeof(HANDLE),
                   &BytesRead, (LPOVERLAPPED) NULL)
         || (BytesRead != sizeof(HANDLE))) {
@@ -493,7 +499,6 @@ static int send_handles_to_child(apr_pool_t *p,
 void get_listeners_from_parent(server_rec *s)
 {
     WSAPROTOCOL_INFO WSAProtocolInfo;
-    HANDLE pipe;
     ap_listen_rec *lr;
     DWORD BytesRead;
     int lcnt = 0;
@@ -510,9 +515,10 @@ void get_listeners_from_parent(server_rec *s)
 
     /* Open the pipe to the parent process to receive the inherited socket
      * data. The sockets have been set to listening in the parent process.
+     *
+     * *** We now do this was back in winnt_rewrite_args
+     * pipe = GetStdHandle(STD_INPUT_HANDLE);
      */
-    pipe = GetStdHandle(STD_INPUT_HANDLE);
-
     for (lr = ap_listeners; lr; lr = lr->next, ++lcnt) {
         if (!ReadFile(pipe, &WSAProtocolInfo, sizeof(WSAPROTOCOL_INFO),
                       &BytesRead, (LPOVERLAPPED) NULL)) {
@@ -618,8 +624,6 @@ static int create_process(apr_pool_t *p, HANDLE *child_proc, HANDLE *child_exit_
     apr_status_t rv;
     apr_pool_t *ptemp;
     apr_procattr_t *attr;
-    apr_file_t *child_out;
-    apr_file_t *child_err;
     apr_proc_t new_child;
     HANDLE hExitEvent;
     HANDLE waitlist[2];  /* see waitlist_e */
@@ -674,36 +678,6 @@ static int create_process(apr_pool_t *p, HANDLE *child_proc, HANDLE *child_exit_
         return -1;
     }
 
-    /* Open a null handle to soak info from the child */
-    if (((rv = apr_file_open(&child_out, "NUL", APR_READ | APR_WRITE,
-                             APR_OS_DEFAULT, ptemp)) != APR_SUCCESS)
-        || ((rv = apr_procattr_child_out_set(attr, child_out, NULL))
-                != APR_SUCCESS)) {
-        ap_log_error(APLOG_MARK, APLOG_CRIT, rv, ap_server_conf,
-                        "Parent: Unable to connect child stdout to NUL.");
-        apr_pool_destroy(ptemp);
-        return -1;
-    }
-
-    /* Connect the child's initial stderr to our main server error log
-     * or share our own stderr handle.
-     */
-    if (ap_server_conf->error_log) {
-        child_err = ap_server_conf->error_log;
-    }
-    else {
-        rv = apr_file_open_stderr(&child_err, ptemp);
-    }
-    if (rv == APR_SUCCESS) {
-        if ((rv = apr_procattr_child_err_set(attr, child_err, NULL))
-                != APR_SUCCESS) {
-            ap_log_error(APLOG_MARK, APLOG_CRIT, rv, ap_server_conf,
-                            "Parent: Unable to connect child stderr.");
-            apr_pool_destroy(ptemp);
-            return -1;
-        }
-    }
-
     /* Create the child_ready_event */
     waitlist[waitlist_ready] = CreateEvent(NULL, TRUE, FALSE, NULL);
     if (!waitlist[waitlist_ready]) {
@@ -802,6 +776,8 @@ static int create_process(apr_pool_t *p, HANDLE *child_proc, HANDLE *child_exit_
         return -1;
     }
 
+    apr_file_close(new_child.in);
+
     *child_exit_event = hExitEvent;
     *child_proc = new_child.hproc;
     *child_pid = new_child.pid;
@@ -1105,6 +1081,9 @@ void winnt_rewrite_args(process_rec *process)
     pid = getenv("AP_PARENT_PID");
     if (pid)
     {
+        HANDLE filehand, newhand;
+        HANDLE hproc = GetCurrentProcess();
+
         /* This is the child */
         my_pid = GetCurrentProcessId();
         parent_pid = (DWORD) atol(pid);
@@ -1112,6 +1091,36 @@ void winnt_rewrite_args(process_rec *process)
         /* Prevent holding open the (nonexistant) console */
         ap_real_exit_code = 0;
 
+        /* The parent gave us stdin, we need to remember this
+         * handle, and no longer inherit it at our children
+         * (we can't slurp it up now, we just aren't ready yet).
+         */
+        pipe = GetStdHandle(STD_INPUT_HANDLE);
+
+        if (osver.dwPlatformId == VER_PLATFORM_WIN32_NT) {
+            /* This doesn't work for 9x, but it's cleaner. */
+            SetHandleInformation(pipe, HANDLE_FLAG_INHERIT, 0);
+        }
+        else if (DuplicateHandle(hproc, pipe,
+                                 hproc, &filehand, 0, FALSE,
+                                 DUPLICATE_SAME_ACCESS)) {
+            CloseHandle(pipe);
+            pipe = filehand;
+        }
+
+        /* The parent gave us stdout of the NUL device,
+         * and expects us to suck up stdin of all of our
+         * shared handles and data from the parent.
+         * Don't infect child processes with our stdin
+         * handle, use another handle to NUL!
+         */
+        if ((filehand = GetStdHandle(STD_OUTPUT_HANDLE))
+            && DuplicateHandle(hproc, filehand, 
+                               hproc, &newhand, 0,
+                               TRUE, DUPLICATE_SAME_ACCESS)) {
+            SetStdHandle(STD_INPUT_HANDLE, newhand);
+        }
+
         /* The parent is responsible for providing the
          * COMPLETE ARGUMENTS REQUIRED to the child.
          *
@@ -1258,6 +1267,8 @@ void winnt_rewrite_args(process_rec *process)
          */
         if (osver.dwPlatformId == VER_PLATFORM_WIN32_NT)
         {
+            apr_file_t *nullfile;
+
             if (!errout) {
                 mpm_nt_eventlog_stderr_open(service_name, process->pool);
             }
@@ -1266,6 +1277,30 @@ void winnt_rewrite_args(process_rec *process)
             if (service_to_start_success == APR_SUCCESS) {
                 service_set = APR_SUCCESS;
             }
+
+            /* Open a null handle to soak stdout in this process.
+             * Windows service processes are missing any file handle
+             * usable for stdin/out/err.  This was the cause of later 
+             * trouble with invocations of apr_file_open_stdout()
+             */
+            if ((rv = apr_file_open(&nullfile, "NUL",
+                                    APR_READ | APR_WRITE, APR_OS_DEFAULT,
+                                    process->pool)) == APR_SUCCESS) {
+                HANDLE hproc = GetCurrentProcess();
+                HANDLE nullstdout = NULL;
+                HANDLE nullhandle;
+
+                /* Duplicate the handle to be inherited by children */
+                if ((apr_os_file_get(&nullhandle, nullfile) == APR_SUCCESS)
+                    && DuplicateHandle(hproc, nullhandle,
+                                       hproc, &nullstdout,
+                                       0, TRUE, DUPLICATE_SAME_ACCESS)) {
+                    SetStdHandle(STD_OUTPUT_HANDLE, nullstdout);
+                }
+
+                /* Close the original handle, we used the duplicate */
+                apr_file_close(nullfile);
+            }
         }
     }
 
@@ -1604,6 +1639,9 @@ static void winnt_child_init(apr_pool_t *pchild, struct server_rec *s)
         /* Set up the listeners */
         get_listeners_from_parent(s);
 
+        /* Done reading from the parent, close that channel */
+        CloseHandle(pipe);
+
         ap_my_generation = ap_scoreboard_image->global->running_generation;
     }
     else {