You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Rob Saccoccio <ro...@fastcgi.com> on 2002/07/18 06:46:54 UTC
[PATCH] apr/threadproc/win32/proc.c:apr_proc_create()
The HANDLE members in the STARTUPINFO struct used in the call to
CreateProcess() aren't currently initialized properly...
--rob
diff -u -r1.80 proc.c
--- proc.c 5 Jul 2002 17:58:10 -0000 1.80
+++ proc.c 18 Jul 2002 04:31:54 -0000
@@ -590,6 +590,10 @@
memset(&si, 0, sizeof(si));
si.cb = sizeof(si);
+ si.hStdInput = INVALID_HANDLE_VALUE;
+ si.hStdOutput = INVALID_HANDLE_VALUE;
+ si.hStdError = INVALID_HANDLE_VALUE;
+
if (attr->detached) {
si.dwFlags |= STARTF_USESHOWWINDOW;
si.wShowWindow = SW_HIDE;
RE: [PATCH] apr/threadproc/win32/proc.c:apr_proc_create()
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 08:15 PM 7/18/2002, Rob Saccoccio wrote:
> > And what was the side-effect of NULL handles to stdout/stderr
> > that you observed? That's what I've been trying to determine.
>
>Oh, sorry. There was no apparent side effect. The resulting process had
>stdout/stderr HANDLEs of 0 as one would expect.
>
>If they're not set to INVALID_HANDLE_VALUE then you've no way to know that
>they're not real.
Actually, you better expect to look for NULL.
If you are writing service applications, the handles are ALL NULL when
your process starts on Win2K/WinXP. So there really was no effective
difference, since [at least service] apps already have this issue.
But since that doesn't conflict badly with INVALID_HANDLE_VALUE,
I'm happy to let the patch stay in. Thank you for submitting it, and
next time around, try to clue us in upfront the observed v.s. expected
behavior problem you had observed was :-)
>While I've got you on the line, why do you do the dup() (in
>apr_procattr_child_in_set()) and close() (in apr_proc_create())?
To toggle the INHERITED attribute of the files. I'm not suggesting
it's toggled correctly [no time to look at the moment] but recognize
that we are trying to create handles as non-inherited by default, but
need to promote them to INHERITED flavors to pass between other
processes.
Bill
RE: [PATCH] apr/threadproc/win32/proc.c:apr_proc_create()
Posted by Rob Saccoccio <ro...@fastcgi.com>.
> And what was the side-effect of NULL handles to stdout/stderr
> that you observed? That's what I've been trying to determine.
Oh, sorry. There was no apparent side effect. The resulting process had
stdout/stderr HANDLEs of 0 as one would expect.
If they're not set to INVALID_HANDLE_VALUE then you've no way to know that
they're not real.
I could have fooled the existing API into setting them up by creating and
passing bogus apr_file_ts (created with apr_os_file_put()) for stdout and
stderr but that doesn't make much sense.
Your patch is perfect/appropriate.
> Also, which version of Win32?
Win2KSP2
While I've got you on the line, why do you do the dup() (in
apr_procattr_child_in_set()) and close() (in apr_proc_create())?
--rob
RE: [PATCH] apr/threadproc/win32/proc.c:apr_proc_create()
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 07:12 PM 7/18/2002, Rob Saccoccio wrote:
> > Please clarify what mischief the NULL handles caused you, Rob.
>
>I was invoking apr_proc_create() with just a stdin handle.
And what was the side-effect of NULL handles to stdout/stderr
that you observed? That's what I've been trying to determine.
Also, which version of Win32?
Bill
RE: [PATCH] apr/threadproc/win32/proc.c:apr_proc_create()
Posted by Rob Saccoccio <ro...@fastcgi.com>.
> Um... -1 on committing this sort of code until I we have some explanation
> of what problem the patch attempted to solve. There is a specific meaning
> to NULL [default to the parent's handles] ... we need to know the behavior
> problem Rob was experiencing.
Yes, but STARTF_USESTDHANDLES was set and those members were NULL.
> There is a better patch to this I'd just committed, that will presume
> it's an all-or-nothing game, you specify all handles and the process
> will have only the handles you define, or specify none and all is well.
Your patch is better/fine. Thanks.
> Please clarify what mischief the NULL handles caused you, Rob.
I was invoking apr_proc_create() with just a stdin handle.
--rob
Re: [PATCH] apr/threadproc/win32/proc.c:apr_proc_create()
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 07:59 AM 7/18/2002, Ian Holsman wrote:
>Rob Saccoccio wrote:
>>The HANDLE members in the STARTUPINFO struct used in the call to
>>CreateProcess() aren't currently initialized properly...
>>--rob
>
>Commited
>Thanks Rob.
Um... -1 on committing this sort of code until I we have some explanation
of what problem the patch attempted to solve. There is a specific meaning
to NULL [default to the parent's handles] ... we need to know the behavior
problem Rob was experiencing.
There is a better patch to this I'd just committed, that will presume
it's an all-or-nothing game, you specify all handles and the process
will have only the handles you define, or specify none and all is well.
Please clarify what mischief the NULL handles caused you, Rob.
Bill
Re: [PATCH] apr/threadproc/win32/proc.c:apr_proc_create()
Posted by Ian Holsman <ia...@apache.org>.
Rob Saccoccio wrote:
> The HANDLE members in the STARTUPINFO struct used in the call to
> CreateProcess() aren't currently initialized properly...
>
> --rob
Commited
Thanks Rob.
>
>
> diff -u -r1.80 proc.c
> --- proc.c 5 Jul 2002 17:58:10 -0000 1.80
> +++ proc.c 18 Jul 2002 04:31:54 -0000
> @@ -590,6 +590,10 @@
>
> memset(&si, 0, sizeof(si));
> si.cb = sizeof(si);
> + si.hStdInput = INVALID_HANDLE_VALUE;
> + si.hStdOutput = INVALID_HANDLE_VALUE;
> + si.hStdError = INVALID_HANDLE_VALUE;
> +
> if (attr->detached) {
> si.dwFlags |= STARTF_USESHOWWINDOW;
> si.wShowWindow = SW_HIDE;
>