You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by rp...@apache.org on 2007/10/05 11:08:13 UTC
svn commit: r582149 - /httpd/httpd/branches/2.2.x/STATUS
Author: rpluem
Date: Fri Oct 5 02:08:13 2007
New Revision: 582149
URL: http://svn.apache.org/viewvc?rev=582149&view=rev
Log:
* Add a comment
Modified:
httpd/httpd/branches/2.2.x/STATUS
Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=582149&r1=582148&r2=582149&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Fri Oct 5 02:08:13 2007
@@ -185,6 +185,10 @@
Not in or needed at trunk/, as apr 1.3.0 has the proper fix.
http://people.apache.org/~wrowe/httpd-2.0-2.2-procattr-bugfix-log.c.patch
+1: wrowe
+ rpluem says: Is this really the correct thing to do on UNIX? I am not sure
+ if all dup2 implementation notice that both fd's are the same. Otherwise
+ they close stdout/stderr first and dup a then closed fd in stdout/stderr,
+ leaving us without stdout/stderr in the child.
* mod_proxy: Fix persistent backend connections.
PR 43472
Re: procattr stuff for 2.2.x
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
> 3) we can restore the pre-2.2.4 behaviour on Win32 by doing something
> simple like the below, without having to mess with the APR procattr code
> or continue adding complexity to the log.c code?
-1 not acceptable, you are looping back to dangling pipes, lingering
processes, non-running code, etc. 2.2.4 and prior code didn't even
run (piped loggers) on win32, remember? Unix had it's own issues.
The 2.2.4-and-prior code was not acceptable, the proposed hack
is similarly unacceptable. It was flawed on so many levels, the
proposed patch is as close as I can come to a comprise given the
broken state of APR (due entirely to your myopic interpretation
of the versioning policy, which I'm through arguing.)
Understand that is a /proposal/, it's still going to take me a few
days to resolve a test/aprpipe.c fault on win32 (one of three on
win32, of which the other two are well-known waiting on apr 2.0).
Actually about 8 last hours of work to fix that and vet this patch
as thoroughly as the first rounds, but I'm distracted thurs/fri
and I'll see how much attention I can apply to it before the
weekend.
I appreciate your desire to avoid "polluting" the unix path, but
it's really senseless to optimize code that's only invoked once/
process, and incredibly destructive to keep altering the code
paths as we saw over the past few months. Keep a single impl.
We have it right in 1.3.0 and I've already flipped the 'apr 1.3
required' bits in httpd trunk. Remember this audit already had
uncloaked a host of UNIX issues. One code path.
If the proposed patch leaves dangling handles, it's because this
is a draft that I haven't finished vetting, I just wanted to get
it out there so folks would have a preview. The final code will
be clean, a few redundant dups in a rarely executed code block,
but clean of dangling pipes on any platform.
> --- server/log.c (revision 583629)
> +++ server/log.c (working copy)
> @@ -265,6 +265,11 @@
> apr_proc_t *procnew;
> apr_file_t *errfile;
>
> +#ifdef WIN32
> + /* workaround for APR 1.2.x */
> + dummy_stderr = 0;
> +#endif
> +
> if (((rc = apr_procattr_create(&procattr, p)) == APR_SUCCESS)
> && ((rc = apr_procattr_cmdtype_set(procattr,
> APR_SHELLCMD_ENV)) == APR_SUCCESS)
>
>
>
procattr stuff for 2.2.x
Posted by Joe Orton <jo...@redhat.com>.
On Fri, Oct 05, 2007 at 09:08:13AM -0000, rpluem@apache.org wrote:
> --- httpd/httpd/branches/2.2.x/STATUS (original)
> +++ httpd/httpd/branches/2.2.x/STATUS Fri Oct 5 02:08:13 2007
> @@ -185,6 +185,10 @@
> Not in or needed at trunk/, as apr 1.3.0 has the proper fix.
> http://people.apache.org/~wrowe/httpd-2.0-2.2-procattr-bugfix-log.c.patch
> +1: wrowe
> + rpluem says: Is this really the correct thing to do on UNIX? I am not sure
> + if all dup2 implementation notice that both fd's are the same. Otherwise
> + they close stdout/stderr first and dup a then closed fd in stdout/stderr,
> + leaving us without stdout/stderr in the child.
Before Bill loses the will to hack the piped logging code... is this all
correct:
1) r452431 introduced a regression on Win32
2) PR 40651 (fixed by r452431) was Unix-specific? Or at least PR 40651
is not as harmful as the aforementioned regression.
3) we can restore the pre-2.2.4 behaviour on Win32 by doing something
simple like the below, without having to mess with the APR procattr code
or continue adding complexity to the log.c code?
Index: server/log.c
===================================================================
--- server/log.c (revision 583629)
+++ server/log.c (working copy)
@@ -265,6 +265,11 @@
apr_proc_t *procnew;
apr_file_t *errfile;
+#ifdef WIN32
+ /* workaround for APR 1.2.x */
+ dummy_stderr = 0;
+#endif
+
if (((rc = apr_procattr_create(&procattr, p)) == APR_SUCCESS)
&& ((rc = apr_procattr_cmdtype_set(procattr,
APR_SHELLCMD_ENV)) == APR_SUCCESS)