apr-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wr...@apache.org
Subject svn commit: r581063 - in /apr/apr/branches/1.2.x: CHANGES file_io/win32/filedup.c file_io/win32/open.c include/apr_thread_proc.h include/arch/win32/apr_arch_file_io.h threadproc/win32/proc.c
Date Mon, 01 Oct 2007 20:22:04 GMT
Author: wrowe
Date: Mon Oct  1 13:22:04 2007
New Revision: 581063

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

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 version 
APR 1.2.x).

Backports: r580484
No backport to later patches for other platforms.

Modified:
    apr/apr/branches/1.2.x/CHANGES
    apr/apr/branches/1.2.x/file_io/win32/filedup.c
    apr/apr/branches/1.2.x/file_io/win32/open.c
    apr/apr/branches/1.2.x/include/apr_thread_proc.h
    apr/apr/branches/1.2.x/include/arch/win32/apr_arch_file_io.h
    apr/apr/branches/1.2.x/threadproc/win32/proc.c

Modified: apr/apr/branches/1.2.x/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/CHANGES?rev=581063&r1=581062&r2=581063&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/CHANGES [utf-8] (original)
+++ apr/apr/branches/1.2.x/CHANGES [utf-8] Mon Oct  1 13:22:04 2007
@@ -1,7 +1,16 @@
-                                                     -*- coding: utf-8 -*-
+                                                     -*- coding: utf-8 -*-
 Changes for APR 1.2.12
 
-
+  *) 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 1.2.10.  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 version APR 1.2.x).
+     [William Rowe]
 
 Changes for APR 1.2.11
 

Modified: apr/apr/branches/1.2.x/file_io/win32/filedup.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/file_io/win32/filedup.c?rev=581063&r1=581062&r2=581063&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/file_io/win32/filedup.c (original)
+++ apr/apr/branches/1.2.x/file_io/win32/filedup.c Mon Oct  1 13:22:04 2007
@@ -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)
@@ -63,10 +65,6 @@
 #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)
 {
@@ -77,32 +75,88 @@
     HANDLE hproc = GetCurrentProcess();
     HANDLE newhand = NULL;
     apr_int32_t newflags;
+    int fd;
 
-    /* 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 (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);
+        }
 
-    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;
     }

Modified: apr/apr/branches/1.2.x/file_io/win32/open.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/file_io/win32/open.c?rev=581063&r1=581062&r2=581063&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/file_io/win32/open.c (original)
+++ apr/apr/branches/1.2.x/file_io/win32/open.c Mon Oct  1 13:22:04 2007
@@ -574,15 +574,11 @@
 
     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
 }
 
@@ -595,15 +591,11 @@
 
     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
 }
 
@@ -616,15 +608,11 @@
 
     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
 }
 

Modified: apr/apr/branches/1.2.x/include/apr_thread_proc.h
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/include/apr_thread_proc.h?rev=581063&r1=581062&r2=581063&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/include/apr_thread_proc.h (original)
+++ apr/apr/branches/1.2.x/include/apr_thread_proc.h Mon Oct  1 13:22:04 2007
@@ -87,6 +87,15 @@
 /** @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 */

Modified: apr/apr/branches/1.2.x/include/arch/win32/apr_arch_file_io.h
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/include/arch/win32/apr_arch_file_io.h?rev=581063&r1=581062&r2=581063&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/include/arch/win32/apr_arch_file_io.h (original)
+++ apr/apr/branches/1.2.x/include/arch/win32/apr_arch_file_io.h Mon Oct  1 13:22:04 2007
@@ -99,6 +99,10 @@
 #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:
  */

Modified: apr/apr/branches/1.2.x/threadproc/win32/proc.c
URL: http://svn.apache.org/viewvc/apr/apr/branches/1.2.x/threadproc/win32/proc.c?rev=581063&r1=581062&r2=581063&view=diff
==============================================================================
--- apr/apr/branches/1.2.x/threadproc/win32/proc.c (original)
+++ apr/apr/branches/1.2.x/threadproc/win32/proc.c Mon Oct  1 13:22:04 2007
@@ -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 @@
             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_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_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_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);
@@ -784,9 +803,10 @@
                     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);
@@ -798,9 +818,10 @@
                     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);
@@ -812,9 +833,10 @@
                     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);
             }
         }
         if (attr->user_token) {
@@ -937,13 +959,13 @@
     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);



Mime
View raw message