You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by wr...@apache.org on 2007/08/25 00:04:24 UTC

svn commit: r569541 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS server/mpm/winnt/mpm_winnt.c

Author: wrowe
Date: Fri Aug 24 15:04:23 2007
New Revision: 569541

URL: http://svn.apache.org/viewvc?rev=569541&view=rev
Log:
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

Modified:
    httpd/httpd/branches/2.2.x/CHANGES
    httpd/httpd/branches/2.2.x/STATUS
    httpd/httpd/branches/2.2.x/server/mpm/winnt/mpm_winnt.c

Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?rev=569541&r1=569540&r2=569541&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Fri Aug 24 15:04:23 2007
@@ -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]

Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=569541&r1=569540&r2=569541&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Fri Aug 24 15:04:23 2007
@@ -290,12 +290,6 @@
       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 

Modified: httpd/httpd/branches/2.2.x/server/mpm/winnt/mpm_winnt.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/mpm/winnt/mpm_winnt.c?rev=569541&r1=569540&r2=569541&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/server/mpm/winnt/mpm_winnt.c (original)
+++ httpd/httpd/branches/2.2.x/server/mpm/winnt/mpm_winnt.c Fri Aug 24 15:04:23 2007
@@ -66,7 +66,6 @@
 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 @@
 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 @@
                              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 *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 @@
 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 @@
 
     /* 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 @@
     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 @@
         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 @@
         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 @@
     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 @@
         /* 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 @@
          */
         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 @@
             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);
+            }
         }
     }
 
@@ -1603,6 +1638,9 @@
 
         /* 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;
     }