You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@apr.apache.org by wr...@apache.org on 2007/09/28 22:57:12 UTC
svn commit: r580484 - in /apr/apr/trunk: 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
Author: wrowe
Date: Fri Sep 28 13:57:11 2007
New Revision: 580484
URL: http://svn.apache.org/viewvc?rev=580484&view=rev
Log:
Introduce APR_NO_FILE as an option for any of the three stdio streams
to cause the specified streams to be closed to the child process,
when the caller has chosen that flag via apr_procattr_io_set().
This is the nonportable flavor targeting 1.2.12; unix 1.3.0 specific
commit to follow.
Modified:
apr/apr/trunk/CHANGES
apr/apr/trunk/file_io/win32/filedup.c
apr/apr/trunk/file_io/win32/open.c
apr/apr/trunk/include/apr_thread_proc.h
apr/apr/trunk/include/arch/win32/apr_arch_file_io.h
apr/apr/trunk/threadproc/win32/proc.c
Modified: apr/apr/trunk/CHANGES
URL: http://svn.apache.org/viewvc/apr/apr/trunk/CHANGES?rev=580484&r1=580483&r2=580484&view=diff
==============================================================================
--- apr/apr/trunk/CHANGES [utf-8] (original)
+++ apr/apr/trunk/CHANGES [utf-8] Fri Sep 28 13:57:11 2007
@@ -1,5 +1,10 @@
- -*- coding: utf-8 -*-
+ -*- coding: utf-8 -*-
Changes for APR 1.3.0
+
+ *) Introduce APR_NO_FILE as an option for any of the three stdio streams
+ to cause the specified streams to be closed to the child process,
+ when the caller has chosen that flag via apr_procattr_io_set().
+ [William Rowe]
*) Solve winNT inherited pipe leaks by mutexing apr_proc_create calls,
on WinNT (not WinCE, nor 9x) so that we toggle the inherited state
Modified: apr/apr/trunk/file_io/win32/filedup.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/filedup.c?rev=580484&r1=580483&r2=580484&view=diff
==============================================================================
--- apr/apr/trunk/file_io/win32/filedup.c (original)
+++ apr/apr/trunk/file_io/win32/filedup.c Fri Sep 28 13:57:11 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/trunk/file_io/win32/open.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/file_io/win32/open.c?rev=580484&r1=580483&r2=580484&view=diff
==============================================================================
--- apr/apr/trunk/file_io/win32/open.c (original)
+++ apr/apr/trunk/file_io/win32/open.c Fri Sep 28 13:57:11 2007
@@ -550,8 +550,7 @@
/* ### check return codes */
(void) apr_pollset_create(&(*file)->pollset, 1, pool, 0);
- /* XXX... we pcalloc above so all others are zeroed.
- * Should we be testing if thefile is a handle to
+ /* Should we be testing if thefile is a handle to
* a PIPE and set up the mechanics appropriately?
*
* (*file)->pipe;
@@ -578,15 +577,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, flags | APR_WRITE, pool);
+ return apr_os_file_put(thefile, &file_handle,
+ flags | APR_WRITE | APR_STDERR_FLAG, pool);
#endif
}
@@ -601,15 +596,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, flags | APR_WRITE, pool);
+ return apr_os_file_put(thefile, &file_handle,
+ flags | APR_WRITE | APR_STDOUT_FLAG, pool);
#endif
}
@@ -624,15 +615,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, flags | APR_READ, pool);
+ return apr_os_file_put(thefile, &file_handle,
+ flags | APR_READ | APR_STDIN_FLAG, pool);
#endif
}
Modified: apr/apr/trunk/include/apr_thread_proc.h
URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/apr_thread_proc.h?rev=580484&r1=580483&r2=580484&view=diff
==============================================================================
--- apr/apr/trunk/include/apr_thread_proc.h (original)
+++ apr/apr/trunk/include/apr_thread_proc.h Fri Sep 28 13:57:11 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/trunk/include/arch/win32/apr_arch_file_io.h
URL: http://svn.apache.org/viewvc/apr/apr/trunk/include/arch/win32/apr_arch_file_io.h?rev=580484&r1=580483&r2=580484&view=diff
==============================================================================
--- apr/apr/trunk/include/arch/win32/apr_arch_file_io.h (original)
+++ apr/apr/trunk/include/arch/win32/apr_arch_file_io.h Fri Sep 28 13:57:11 2007
@@ -99,8 +99,12 @@
#define APR_OPENINFO 0x00100000 /* Open without READ or WRITE access */
#define APR_OPENLINK 0x00200000 /* Open a link itself, if supported */
#define APR_READCONTROL 0x00400000 /* Read the file's owner/perms */
-#define APR_WRITECONTROL 0x00800000 /* Modifythe file's owner/perms */
+#define APR_WRITECONTROL 0x00800000 /* Modify the 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/trunk/threadproc/win32/proc.c
URL: http://svn.apache.org/viewvc/apr/apr/trunk/threadproc/win32/proc.c?rev=580484&r1=580483&r2=580484&view=diff
==============================================================================
--- apr/apr/trunk/threadproc/win32/proc.c (original)
+++ apr/apr/trunk/threadproc/win32/proc.c Fri Sep 28 13:57:11 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
@@ -50,12 +59,12 @@
#define SW_HIDE 0
#endif
#endif
+
/*
* some of the ideas expressed herein are based off of Microsoft
* Knowledge Base article: Q190351
*
*/
-
APR_DECLARE(apr_status_t) apr_procattr_create(apr_procattr_t **new,
apr_pool_t *pool)
{
@@ -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);