Backport r580484; the patchset to teach APR dup2 to hook up stdio

to MSVCRT on Win32, and to provide a back-compatibility hook-up
for users who positively, absolutely need INVALID_HANDLE_VALUES.

git-svn-id: http://svn.apache.org/repos/asf/apr/apr/branches/0.9.x@581110 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 164 insertions, 75 deletions

diff --git a/CHANGES b/CHANGES
index e18f79b6d..5748450d4 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,7 +1,19 @@
-                                                     -*- coding: utf-8 -*-
+                                                     -*- coding: utf-8 -*-
 Changes with APR 0.9.17
 
-
+  *) Cause apr_file_dup2() on Win32 to update the MSVCRT psuedo-stdio
+     handles for fd-based and FILE * based I/O.  [William Rowe]
+
+  *) Introduce APR_NO_FILE as an option to apr_procattr_io_set() for any 
+     of the three stdio streams to cause the corresponding streams to NOT
+     be inherited to a WIN32 child process (passes INVALID_HANDLE_VALUE).
+     This is of little importance to most developers, except those who had
+     exploited an inconsistency between Unix and Win32 that was corrected
+     on Win32 with APR version 0.9.16.  The feature to have NO_FILE work
+     across platforms and leave a specified std stream closed becomes 
+     portable in APR version 1.3.0 (it remains equivilant to APR_NO_PIPE 
+     on non-Windows platforms throughout APR versions 0.9.x and 1.2.x).
+     [William Rowe]
 
 Changes with APR 0.9.16
 
diff --git a/file_io/win32/filedup.c b/file_io/win32/filedup.c
index 7568b7407..039f2ccd6 100644
--- a/file_io/win32/filedup.c
+++ b/file_io/win32/filedup.c
@@ -20,6 +20,8 @@
 #include "apr_strings.h"
 #include <string.h>
 #include "apr_arch_inherit.h"
+#include <io.h> /* for [_open/_get]_osfhandle */
+
 
 APR_DECLARE(apr_status_t) apr_file_dup(apr_file_t **new_file,
                                        apr_file_t *old_file, apr_pool_t *p)
@@ -59,10 +61,6 @@ APR_DECLARE(apr_status_t) apr_file_dup(apr_file_t **new_file,
 #endif /* !defined(_WIN32_WCE) */
 }
 
-#define stdin_handle 0x01
-#define stdout_handle 0x02
-#define stderr_handle 0x04
-
 APR_DECLARE(apr_status_t) apr_file_dup2(apr_file_t *new_file,
                                         apr_file_t *old_file, apr_pool_t *p)
 {
@@ -73,32 +71,88 @@ APR_DECLARE(apr_status_t) apr_file_dup2(apr_file_t *new_file,
     HANDLE hproc = GetCurrentProcess();
     HANDLE newhand = NULL;
     apr_int32_t newflags;
+    int fd;
+
+    if (new_file->flags & APR_STD_FLAGS) {
+        /* if (!DuplicateHandle(hproc, old_file->filehand, 
+         *                      hproc, &newhand, 0,
+         *                      TRUE, DUPLICATE_SAME_ACCESS)) {
+         *     return apr_get_os_error();
+         * }
+         * if (((stdhandle & stderr_handle) && !SetStdHandle(STD_ERROR_HANDLE, newhand)) ||
+         *     ((stdhandle & stdout_handle) && !SetStdHandle(STD_OUTPUT_HANDLE, newhand)) ||
+         *     ((stdhandle & stdin_handle) && !SetStdHandle(STD_INPUT_HANDLE, newhand))) {
+         *     return apr_get_os_error();
+         * }
+         * newflags = old_file->flags | APR_INHERIT;
+         */
+
+        if ((new_file->flags & APR_STD_FLAGS) == APR_STDERR_FLAG) {
+            /* Flush stderr and unset its buffer, then commit the fd-based buffer.
+             * This is typically a noop for Win2K/XP since services with NULL std
+             * handles [but valid FILE *'s, oddly enough], but is required
+             * for NT 4.0 and to use this code outside of services.
+             */
+            fflush(stderr);
+            setvbuf(stderr, NULL, _IONBF, 0);
+            _commit(2 /* stderr */);
+
+            /* Clone a handle can _close() without harming the source handle,
+             * open an MSVCRT-based pseudo-fd for the file handle, then dup2
+             * and close our temporary pseudo-fd once it's been duplicated.
+             * This will incidently keep the FILE-based stderr in sync.
+             * Note the apparently redundant _O_BINARY coersions are required.
+             */
+            if (!DuplicateHandle(hproc, old_file->filehand, hproc, &newhand,
+                                 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+                return apr_get_os_error();
+            }
+            fd = _open_osfhandle((INT_PTR)newhand, _O_WRONLY | _O_BINARY);
+            _dup2(fd, 2);
+            _close(fd);
+            _setmode(2, _O_BINARY);
+
+            /* hPipeWrite was _close()'ed above, and _dup2()'ed
+             * to fd 2 creating a new, inherited Win32 handle.
+             * Recover that real handle from fd 2.  Note that
+             * SetStdHandle(STD_ERROR_HANDLE, _get_osfhandle(2))
+             * is implicit in the dup2() call above
+             */
+            newhand = (HANDLE)_get_osfhandle(2);
+        }
 
-    /* dup2 is not supported literaly with native Windows handles.
-     * We can, however, emulate dup2 for the standard i/o handles,
-     * and close and replace other handles with duped handles.
-     * The os_handle will change, however.
-     */
-    if (new_file->filehand == GetStdHandle(STD_ERROR_HANDLE)) {
-        stdhandle |= stderr_handle;
-    }
-    if (new_file->filehand == GetStdHandle(STD_OUTPUT_HANDLE)) {
-        stdhandle |= stdout_handle;
-    }
-    if (new_file->filehand == GetStdHandle(STD_INPUT_HANDLE)) {
-        stdhandle |= stdin_handle;
-    }
-
-    if (stdhandle) {
-        if (!DuplicateHandle(hproc, old_file->filehand, 
-                             hproc, &newhand, 0,
-                             TRUE, DUPLICATE_SAME_ACCESS)) {
-            return apr_get_os_error();
+        if ((new_file->flags & APR_STD_FLAGS) == APR_STDOUT_FLAG) {
+            /* For the process flow see the stderr case above */
+            fflush(stdout);
+            setvbuf(stdout, NULL, _IONBF, 0);
+            _commit(1 /* stdout */);
+
+            if (!DuplicateHandle(hproc, old_file->filehand, hproc, &newhand,
+                                 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+                return apr_get_os_error();
+            }
+            fd = _open_osfhandle((INT_PTR)newhand, _O_WRONLY | _O_BINARY);
+            _dup2(fd, 1);
+            _close(fd);
+            _setmode(1, _O_BINARY);
+            newhand = (HANDLE)_get_osfhandle(1);
         }
-        if (((stdhandle & stderr_handle) && !SetStdHandle(STD_ERROR_HANDLE, newhand)) ||
-            ((stdhandle & stdout_handle) && !SetStdHandle(STD_OUTPUT_HANDLE, newhand)) ||
-            ((stdhandle & stdin_handle) && !SetStdHandle(STD_INPUT_HANDLE, newhand))) {
-            return apr_get_os_error();
+
+        if ((new_file->flags & APR_STD_FLAGS) == APR_STDIN_FLAG) {
+            /* For the process flow see the stderr case above */
+            fflush(stdin);
+            setvbuf(stdin, NULL, _IONBF, 0);
+            _commit(0 /* stdin */);
+
+            if (!DuplicateHandle(hproc, old_file->filehand, hproc, &newhand,
+                                 0, FALSE, DUPLICATE_SAME_ACCESS)) {
+                return apr_get_os_error();
+            }
+            fd = _open_osfhandle((INT_PTR)newhand, _O_RDONLY | _O_BINARY);
+            _dup2(fd, 0);
+            _close(fd);
+            _setmode(0, _O_BINARY);
+            newhand = (HANDLE)_get_osfhandle(0);
         }
         newflags = old_file->flags | APR_INHERIT;
     }
diff --git a/file_io/win32/open.c b/file_io/win32/open.c
index b3b5d3f5d..6be045f4c 100644
--- a/file_io/win32/open.c
+++ b/file_io/win32/open.c
@@ -566,15 +566,11 @@ APR_DECLARE(apr_status_t) apr_file_open_stderr(apr_file_t **thefile, apr_pool_t
 
     apr_set_os_error(APR_SUCCESS);
     file_handle = GetStdHandle(STD_ERROR_HANDLE);
-    if (!file_handle || (file_handle == INVALID_HANDLE_VALUE)) {
-        apr_status_t rv = apr_get_os_error();
-        if (rv == APR_SUCCESS) {
-            return APR_EINVAL;
-        }
-        return rv;
-    }
+    if (!file_handle)
+        file_handle = INVALID_HANDLE_VALUE;
 
-    return apr_os_file_put(thefile, &file_handle, 0, pool);
+    return apr_os_file_put(thefile, &file_handle,
+                           APR_WRITE | APR_STDERR_FLAG, pool);
 #endif
 }
 
@@ -587,15 +583,11 @@ APR_DECLARE(apr_status_t) apr_file_open_stdout(apr_file_t **thefile, apr_pool_t
 
     apr_set_os_error(APR_SUCCESS);
     file_handle = GetStdHandle(STD_OUTPUT_HANDLE);
-    if (!file_handle || (file_handle == INVALID_HANDLE_VALUE)) {
-        apr_status_t rv = apr_get_os_error();
-        if (rv == APR_SUCCESS) {
-            return APR_EINVAL;
-        }
-        return rv;
-    }
+    if (!file_handle)
+        file_handle = INVALID_HANDLE_VALUE;
 
-    return apr_os_file_put(thefile, &file_handle, 0, pool);
+    return apr_os_file_put(thefile, &file_handle,
+                           APR_WRITE | APR_STDOUT_FLAG, pool);
 #endif
 }
 
@@ -608,15 +600,11 @@ APR_DECLARE(apr_status_t) apr_file_open_stdin(apr_file_t **thefile, apr_pool_t *
 
     apr_set_os_error(APR_SUCCESS);
     file_handle = GetStdHandle(STD_INPUT_HANDLE);
-    if (!file_handle || (file_handle == INVALID_HANDLE_VALUE)) {
-        apr_status_t rv = apr_get_os_error();
-        if (rv == APR_SUCCESS) {
-            return APR_EINVAL;
-        }
-        return rv;
-    }
+    if (!file_handle)
+        file_handle = INVALID_HANDLE_VALUE;
 
-    return apr_os_file_put(thefile, &file_handle, 0, pool);
+    return apr_os_file_put(thefile, &file_handle,
+                           APR_READ | APR_STDIN_FLAG, pool);
 #endif
 }
 
diff --git a/include/apr_thread_proc.h b/include/apr_thread_proc.h
index be586040e..6d1c0be14 100644
--- a/include/apr_thread_proc.h
+++ b/include/apr_thread_proc.h
@@ -87,6 +87,15 @@ typedef enum {
 /** @see apr_procattr_io_set */
 #define APR_CHILD_BLOCK      4
 
+/** @see apr_procattr_io_set 
+ * @note introduced strictly for Win32 to apr revision 1.2.12 (to restore
+ * the non-portable default behavior of 1.2.9 and prior versions on Win32).
+ * This becomes portable to all platforms effective revision 1.3.0, ensuring
+ * the standard files specified in the call to apr_procattr_io_set are not
+ * open in the created process (on Win32 as INVALID_HANDLE_VALUEs).
+ */
+#define APR_NO_FILE          8
+
 /** @see apr_procattr_limit_set */
 #define APR_LIMIT_CPU        0
 /** @see apr_procattr_limit_set */
diff --git a/include/arch/win32/apr_arch_file_io.h b/include/arch/win32/apr_arch_file_io.h
index bcb9d0823..dc3f442a1 100644
--- a/include/arch/win32/apr_arch_file_io.h
+++ b/include/arch/win32/apr_arch_file_io.h
@@ -98,6 +98,10 @@ void *res_name_from_filename(const char *file, int global, apr_pool_t *pool);
 #define APR_READCONTROL  0x00400000 /* Read the file's owner/perms */
 #define APR_WRITECONTROL 0x00800000 /* Modifythe file's owner/perms */
 #define APR_WRITEATTRS   0x01000000 /* Modify the file's attributes */
+#define APR_STDIN_FLAG   0x02000000 /* Obtained via apr_file_open_stdin() */
+#define APR_STDOUT_FLAG  0x04000000 /* Obtained via apr_file_open_stdout() */
+#define APR_STDERR_FLAG  0x06000000 /* Obtained via apr_file_open_stderr() */
+#define APR_STD_FLAGS    (APR_STDIN_FLAG | APR_STDOUT_FLAG | APR_STDERR_FLAG)
 
 /* Entries missing from the MSVC 5.0 Win32 SDK:
  */
diff --git a/threadproc/win32/proc.c b/threadproc/win32/proc.c
index f18173b6c..34a21da9a 100644
--- a/threadproc/win32/proc.c
+++ b/threadproc/win32/proc.c
@@ -32,6 +32,15 @@
 #include <process.h>
 #endif
 
+/* Heavy on no'ops, here's what we want to pass if there is APR_NO_FILE
+ * requested for a specific child handle;
+ */
+static apr_file_t no_file = { NULL, INVALID_HANDLE_VALUE, };
+
+/* We have very carefully excluded volumes of definitions from the
+ * Microsoft Platform SDK, which kill the build time performance.
+ * These the sole constants we borrow from WinBase.h and WinUser.h
+ */
 #ifndef LOGON32_LOGON_NETWORK
 #define LOGON32_LOGON_NETWORK 3
 #endif
@@ -83,20 +92,30 @@ APR_DECLARE(apr_status_t) apr_procattr_io_set(apr_procattr_t *attr,
             in = APR_READ_BLOCK;
         else if (in == APR_PARENT_BLOCK)
             in = APR_WRITE_BLOCK;
-        stat = apr_create_nt_pipe(&attr->child_in, &attr->parent_in, in,
-                                  attr->pool);
+
+        if (in == APR_NO_FILE)
+            attr->child_in = &no_file;
+        else
+            stat = apr_create_nt_pipe(&attr->child_in, &attr->parent_in,
+                                      in, attr->pool);
         if (stat == APR_SUCCESS)
             stat = apr_file_inherit_unset(attr->parent_in);
     }
     if (out && stat == APR_SUCCESS) {
-        stat = apr_create_nt_pipe(&attr->parent_out, &attr->child_out, out,
-                                  attr->pool);
+        if (out == APR_NO_FILE)
+            attr->child_out = &no_file;
+        else
+            stat = apr_create_nt_pipe(&attr->parent_out, &attr->child_out,
+                                      out, attr->pool);
         if (stat == APR_SUCCESS)
             stat = apr_file_inherit_unset(attr->parent_out);
     }
     if (err && stat == APR_SUCCESS) {
-        stat = apr_create_nt_pipe(&attr->parent_err, &attr->child_err, err,
-                                  attr->pool);
+        if (err == APR_NO_FILE)
+            attr->child_err = &no_file;
+        else
+            stat = apr_create_nt_pipe(&attr->parent_err, &attr->child_err,
+                                      err, attr->pool);
         if (stat == APR_SUCCESS)
             stat = apr_file_inherit_unset(attr->parent_err);
     }
@@ -110,7 +129,7 @@ APR_DECLARE(apr_status_t) apr_procattr_child_in_set(apr_procattr_t *attr,
     apr_status_t rv = APR_SUCCESS;
 
     if (child_in) {
-        if (attr->child_in == NULL)
+        if ((attr->child_in == NULL) || (attr->child_in == &no_file))
             rv = apr_file_dup(&attr->child_in, child_in, attr->pool);
         else
             rv = apr_file_dup2(attr->child_in, child_in, attr->pool);
@@ -136,7 +155,7 @@ APR_DECLARE(apr_status_t) apr_procattr_child_out_set(apr_procattr_t *attr,
     apr_status_t rv = APR_SUCCESS;
 
     if (child_out) {
-        if (attr->child_out == NULL)
+        if ((attr->child_out == NULL) || (attr->child_out == &no_file))
             rv = apr_file_dup(&attr->child_out, child_out, attr->pool);
         else
             rv = apr_file_dup2(attr->child_out, child_out, attr->pool);
@@ -162,7 +181,7 @@ APR_DECLARE(apr_status_t) apr_procattr_child_err_set(apr_procattr_t *attr,
     apr_status_t rv = APR_SUCCESS;
 
     if (child_err) {
-        if (attr->child_err == NULL)
+        if ((attr->child_err == NULL) || (attr->child_err == &no_file))
             rv = apr_file_dup(&attr->child_err, child_err, attr->pool);
         else
             rv = apr_file_dup2(attr->child_err, child_err, attr->pool);
@@ -669,9 +688,10 @@ APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
                     SetHandleInformation(si.hStdInput,
                                          HANDLE_FLAG_INHERIT, 0);
 
-                si.hStdInput = attr->child_in->filehand;
-                SetHandleInformation(si.hStdInput, HANDLE_FLAG_INHERIT,
-                                                   HANDLE_FLAG_INHERIT);
+                if ( (si.hStdInput = attr->child_in->filehand) 
+                                   != INVALID_HANDLE_VALUE )
+                    SetHandleInformation(si.hStdInput, HANDLE_FLAG_INHERIT,
+                                                       HANDLE_FLAG_INHERIT);
             }
             
             si.hStdOutput = GetStdHandle(STD_OUTPUT_HANDLE);
@@ -683,9 +703,10 @@ APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
                     SetHandleInformation(si.hStdOutput,
                                          HANDLE_FLAG_INHERIT, 0);
 
-                si.hStdOutput = attr->child_out->filehand;
-                SetHandleInformation(si.hStdOutput, HANDLE_FLAG_INHERIT,
-                                                    HANDLE_FLAG_INHERIT);
+                if ( (si.hStdOutput = attr->child_out->filehand) 
+                                   != INVALID_HANDLE_VALUE )
+                    SetHandleInformation(si.hStdOutput, HANDLE_FLAG_INHERIT,
+                                                        HANDLE_FLAG_INHERIT);
             }
 
             si.hStdError = GetStdHandle(STD_ERROR_HANDLE);
@@ -697,9 +718,10 @@ APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
                     SetHandleInformation(si.hStdError,
                                          HANDLE_FLAG_INHERIT, 0);
 
-                si.hStdError = attr->child_err->filehand;
-                SetHandleInformation(si.hStdError, HANDLE_FLAG_INHERIT,
-                                                   HANDLE_FLAG_INHERIT);
+                if ( (si.hStdError = attr->child_err->filehand) 
+                                   != INVALID_HANDLE_VALUE )
+                    SetHandleInformation(si.hStdError, HANDLE_FLAG_INHERIT,
+                                                       HANDLE_FLAG_INHERIT);
             }
         }
         rv = CreateProcessW(wprg, wcmd,        /* Executable & Command line */
@@ -794,13 +816,13 @@ APR_DECLARE(apr_status_t) apr_proc_create(apr_proc_t *new,
     new->hproc = pi.hProcess;
     new->pid = pi.dwProcessId;
 
-    if (attr->child_in) {
+    if ((attr->child_in) && (attr->child_in != &no_file)) {
         apr_file_close(attr->child_in);
     }
-    if (attr->child_out) {
+    if ((attr->child_out) && (attr->child_out != &no_file)) {
         apr_file_close(attr->child_out);
     }
-    if (attr->child_err) {
+    if ((attr->child_err) && (attr->child_err != &no_file)) {
         apr_file_close(attr->child_err);
     }
     CloseHandle(pi.hThread);