You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Jeff Trawick <tr...@gmail.com> on 2007/10/06 13:40:00 UTC
PR43563 patch for writes to non-blocking pipes on Windows
http://issues.apache.org/bugzilla/show_bug.cgi?id=43563
This patch looks reasonable to me; any thoughts from the Windows crowd?
--
Born in Roswell... married an alien...
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Eric Covener wrote:
> On 10/6/07, Jeff Trawick <tr...@gmail.com> wrote:
>> http://issues.apache.org/bugzilla/show_bug.cgi?id=43563
>>
>> This patch looks reasonable to me; any thoughts from the Windows crowd?
>>
>
> 43522 is a close cousin:
>
> http://issues.apache.org/bugzilla/show_bug.cgi?id=43522
>
> I believe last sentiment from Bill was that this is backportable
> (although it's not in trunk yet)
Yes - the baseline fix of return APR_EAGAIN was critical, but the other
patch would also be desirable to make this behavior consistent to unix,
and it's so trivial to override by setting an explicit timeout.
FYI trunk has one fail-case right now in testdup, investigating.
Bill
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Eric Covener <co...@gmail.com>.
On 10/8/07, Jeff Trawick <tr...@gmail.com> wrote:
> On 10/8/07, Eric Covener <co...@gmail.com> wrote:
> > On 10/6/07, Jeff Trawick <tr...@gmail.com> wrote:
> > > http://issues.apache.org/bugzilla/show_bug.cgi?id=43563
> > >
> > > This patch looks reasonable to me; any thoughts from the Windows crowd?
> > >
> >
> > 43522 is a close cousin:
> >
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=43522
>
> plz save me some time and help me understand why this first part of
> this patch checks APR_READ_BLOCK and APR_WRITE_BLOCK, whereas the part
> of the patch for the other two handles checks APR_PARENT_BLOCK and
> APR_CHILD_BLOCK (the Unix code checks ...PARENT... and ...CHILD... for
> all three handles)
>
> @@ -95,27 +95,75 @@
>
> if (in == APR_NO_FILE)
> attr->child_in = &no_file;
> - else
> + else {
> stat = apr_create_nt_pipe(&attr->child_in, &attr->parent_in,
> in, attr->pool);
> + if (stat == APR_SUCCESS) {
> + switch (in) {
> + case APR_FULL_BLOCK:
> + break;
> + case APR_READ_BLOCK:
> + apr_file_pipe_timeout_set(attr->parent_in, 0);
> + break;
> + case APR_WRITE_BLOCK:
> + apr_file_pipe_timeout_set(attr->child_in, 0);
> + break;
>
> Thanks!
>
The stdin parameter is transposed early in this routine for subsequent
calls to apr_create_nt_pipe() who only speaks in terms of
APR_READ_BLOCK/APR_WRITE_BLOCK and not parent/child.
(and I wanted the patch to be as minimal as possible)
/* APR_CHILD_BLOCK maps to APR_WRITE_BLOCK, while
* APR_PARENT_BLOCK maps to APR_READ_BLOCK, so we
* must transpose the CHILD/PARENT blocking flags
* only for the stdin pipe. stdout/stderr naturally
* map to the correct mode.
*/
if (in == APR_CHILD_BLOCK)
in = APR_READ_BLOCK;
else if (in == APR_PARENT_BLOCK)
in = APR_WRITE_BLOCK;
For stdin, APR_CHILD_BLOCK becomes APR_READ_BLOCK which means the
parent/writer is non-blocking:
> + case APR_READ_BLOCK:
> + apr_file_pipe_timeout_set(attr->parent_in, 0);
For stderr/stdout, APR_CHILD_BLOCK is mapped directly and the
parent/reader is non-blocking:
+ case APR_CHILD_BLOCK:
+ apr_file_pipe_timeout_set(attr->parent_out, 0);
--
Eric Covener
covener@gmail.com
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Jeff Trawick <tr...@gmail.com>.
On 10/8/07, Eric Covener <co...@gmail.com> wrote:
> On 10/6/07, Jeff Trawick <tr...@gmail.com> wrote:
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=43563
> >
> > This patch looks reasonable to me; any thoughts from the Windows crowd?
> >
>
> 43522 is a close cousin:
>
> http://issues.apache.org/bugzilla/show_bug.cgi?id=43522
plz save me some time and help me understand why this first part of
this patch checks APR_READ_BLOCK and APR_WRITE_BLOCK, whereas the part
of the patch for the other two handles checks APR_PARENT_BLOCK and
APR_CHILD_BLOCK (the Unix code checks ...PARENT... and ...CHILD... for
all three handles)
@@ -95,27 +95,75 @@
if (in == APR_NO_FILE)
attr->child_in = &no_file;
- else
+ else {
stat = apr_create_nt_pipe(&attr->child_in, &attr->parent_in,
in, attr->pool);
+ if (stat == APR_SUCCESS) {
+ switch (in) {
+ case APR_FULL_BLOCK:
+ break;
+ case APR_READ_BLOCK:
+ apr_file_pipe_timeout_set(attr->parent_in, 0);
+ break;
+ case APR_WRITE_BLOCK:
+ apr_file_pipe_timeout_set(attr->child_in, 0);
+ break;
Thanks!
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Eric Covener wrote:
>
> I wasn't able to test with httpd/extfilter because of some problems in
> httpd 2.2.x HEAD (w/o above patch)
Twofold; httpd 2.2 branch is not patched (two patches waiting feedback, but
httpd trunk 2.3-dev has been patched) and there is a dup2 bug (see prior post
on dev@apr).
Bill
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Eric Covener <co...@gmail.com>.
On 10/11/07, Jeff Trawick <tr...@gmail.com> wrote:
> On 10/11/07, Eric Covener <co...@gmail.com> wrote:
> > On 10/11/07, Jeff Trawick <tr...@gmail.com> wrote:
> > > On 10/11/07, Eric Covener <co...@gmail.com> wrote:
> > > >
> > > > http://people.apache.org/~covener/1.2.x-win32-pipetimeout.diff
> > >
> > > thanks; committed to 1.2.x with r583860
> > >
> >
> > I will resurrect this thread after 1.2.next is shipped; would like to
> > see at least 43563 (write->APR_EAGAIN) in 0.9.x for mod_ext_filter in
> > httpd 2.0.x
> >
> > (43522 would be nice, but can workaround by being explicit in caller)
>
> have patches? 43563 is surely the same patch; is 0.9.x close enough
> to 1.2.x now w.r.t. 43522?
>
1.2.x patch for 43522 applies cleanly to 0.9.x
--
Eric Covener
covener@gmail.com
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Jeff Trawick <tr...@gmail.com>.
On 10/11/07, Eric Covener <co...@gmail.com> wrote:
> On 10/11/07, Jeff Trawick <tr...@gmail.com> wrote:
> > On 10/11/07, Eric Covener <co...@gmail.com> wrote:
> > >
> > > http://people.apache.org/~covener/1.2.x-win32-pipetimeout.diff
> >
> > thanks; committed to 1.2.x with r583860
> >
>
> I will resurrect this thread after 1.2.next is shipped; would like to
> see at least 43563 (write->APR_EAGAIN) in 0.9.x for mod_ext_filter in
> httpd 2.0.x
>
> (43522 would be nice, but can workaround by being explicit in caller)
have patches? 43563 is surely the same patch; is 0.9.x close enough
to 1.2.x now w.r.t. 43522?
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Eric Covener <co...@gmail.com>.
On 10/11/07, Jeff Trawick <tr...@gmail.com> wrote:
> On 10/11/07, Eric Covener <co...@gmail.com> wrote:
> >
> > http://people.apache.org/~covener/1.2.x-win32-pipetimeout.diff
>
> thanks; committed to 1.2.x with r583860
>
I will resurrect this thread after 1.2.next is shipped; would like to
see at least 43563 (write->APR_EAGAIN) in 0.9.x for mod_ext_filter in
httpd 2.0.x
(43522 would be nice, but can workaround by being explicit in caller)
--
Eric Covener
covener@gmail.com
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Jeff Trawick <tr...@gmail.com>.
On 10/11/07, Eric Covener <co...@gmail.com> wrote:
> On 10/10/07, Jeff Trawick <tr...@gmail.com> wrote:
> > It's in trunk now.
> >
> > Do you have a patch for the 1.2.x branch, which has diverged with the
> > segregation of APR_NO_FILE?
>
> http://people.apache.org/~covener/1.2.x-win32-pipetimeout.diff
thanks; committed to 1.2.x with r583860
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Eric Covener <co...@gmail.com>.
On 10/10/07, Jeff Trawick <tr...@gmail.com> wrote:
> It's in trunk now.
>
> Do you have a patch for the 1.2.x branch, which has diverged with the
> segregation of APR_NO_FILE?
http://people.apache.org/~covener/1.2.x-win32-pipetimeout.diff
I wasn't able to test with httpd/extfilter because of some problems in
httpd 2.2.x HEAD (w/o above patch)
[Thu Oct 11 10:53:02 2007] [crit] (OS 232)The pipe is being closed. :
Parent: Unable to send the exit event handle to the child
[Thu Oct 11 10:53:02 2007] [crit] (OS 6)The handle is invalid. :
master_main: create child process failed. Exiting.
(I assumed this is due to all the recent activity in this neighborhood)
There was no regression in apr/test/testall after the patch (win2k3/vc6)
--
Eric Covener
covener@gmail.com
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Jeff Trawick <tr...@gmail.com>.
On 10/8/07, Eric Covener <co...@gmail.com> wrote:
> On 10/6/07, Jeff Trawick <tr...@gmail.com> wrote:
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=43563
> >
> > This patch looks reasonable to me; any thoughts from the Windows crowd?
> >
>
> 43522 is a close cousin:
>
> http://issues.apache.org/bugzilla/show_bug.cgi?id=43522
>
> I believe last sentiment from Bill was that this is backportable
> (although it's not in trunk yet)
It's in trunk now.
Do you have a patch for the 1.2.x branch, which has diverged with the
segregation of APR_NO_FILE?
Thanks!
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Jeff Trawick <tr...@gmail.com>.
On 10/8/07, Eric Covener <co...@gmail.com> wrote:
> On 10/6/07, Jeff Trawick <tr...@gmail.com> wrote:
> > http://issues.apache.org/bugzilla/show_bug.cgi?id=43563
> >
> > This patch looks reasonable to me; any thoughts from the Windows crowd?
> >
>
> 43522 is a close cousin:
>
> http://issues.apache.org/bugzilla/show_bug.cgi?id=43522
>
> I believe last sentiment from Bill was that this is backportable
> (although it's not in trunk yet)
I'll try to get both of these in trunk and 1.2.x tonight when I have
some time and my favorite checkouts handy, unless somebody beats me
[to it] before then.
Re: PR43563 patch for writes to non-blocking pipes on Windows
Posted by Eric Covener <co...@gmail.com>.
On 10/6/07, Jeff Trawick <tr...@gmail.com> wrote:
> http://issues.apache.org/bugzilla/show_bug.cgi?id=43563
>
> This patch looks reasonable to me; any thoughts from the Windows crowd?
>
43522 is a close cousin:
http://issues.apache.org/bugzilla/show_bug.cgi?id=43522
I believe last sentiment from Bill was that this is backportable
(although it's not in trunk yet)
--
Eric Covener
covener@gmail.com