httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ryan Morgan <rmor...@covalent.net>
Subject Re: [PATCH] core_output_filter
Date Sun, 22 Sep 2002 03:46:19 GMT

Thats a good point, I didn't think about that case.  Maybe the safe way is
to just add a check for APR_TIMEUP?

Index: core.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/core.c,v
retrieving revision 1.205
diff -u -r1.205 core.c
--- core.c	16 Sep 2002 13:11:55 -0000	1.205
+++ core.c	22 Sep 2002 03:44:25 -0000
@@ -3912,7 +3912,8 @@
 
             if (APR_STATUS_IS_ECONNABORTED(rv)
                 || APR_STATUS_IS_ECONNRESET(rv)
-                || APR_STATUS_IS_EPIPE(rv)) {
+                || APR_STATUS_IS_EPIPE(rv)
+                || APR_STATUS_IS_TIMEUP(rv)) {
                 c->aborted = 1;
             }
 
-Ryan

On Sat, Sep 21, 2002 at 11:42:22AM -0400, Bill Stoddard wrote:
> This will definitely break when we try to do non-blocking sends which will
> return APR_EAGAIN
> Bill
> 
> > I have a quick question about some of the logic in the core_output_filter.
> >
> > When the core_output_filter sends data to the network (via writev_it_all,
> > emulate_sendfile, or sendfile_it_all), we check the apr_status_t and log
> > an error when rv != APR_SUCCESS.  However, apache only sets c->aborted if
> > the status code was ECONNABORTED, ECONNRESET, or EPIPE.
> >
> > Is there a reason we only set c->aborted for these specific
> > return values?
> > I think it may make more sense to always set c->aborted if we
> > encounter an
> > error.  This flag is the only way modules will know if something in the
> > filter chain went wrong.
> >
> > A patch is attached to always set c->aborted on network send errors.
> > (although I have only run into this problem with APR_TIMEUP errors, so
> >  we could also just add a APR_STATUS_IS_TIMEUP(rv) check)
> >
> > -Ryan
> >
> > Index: core.c
> > ===================================================================
> > RCS file: /home/cvspublic/httpd-2.0/server/core.c,v
> > retrieving revision 1.205
> > diff -u -r1.205 core.c
> > --- core.c      16 Sep 2002 13:11:55 -0000      1.205
> > +++ core.c      20 Sep 2002 20:46:52 -0000
> > @@ -3910,11 +3910,7 @@
> >              if (more)
> >                  apr_brigade_destroy(more);
> >
> > -            if (APR_STATUS_IS_ECONNABORTED(rv)
> > -                || APR_STATUS_IS_ECONNRESET(rv)
> > -                || APR_STATUS_IS_EPIPE(rv)) {
> > -                c->aborted = 1;
> > -            }
> > +            c->aborted = 1;
> >
> >              /* The client has aborted, but the request was successful. We
> >               * will report success, and leave it to the access and error
> >
> 
> 


-Ryan

On Sat, Sep 21, 2002 at 11:42:22AM -0400, Bill Stoddard wrote:
> This will definitely break when we try to do non-blocking sends which will
> return APR_EAGAIN
> Bill
> 
> > I have a quick question about some of the logic in the core_output_filter.
> >
> > When the core_output_filter sends data to the network (via writev_it_all,
> > emulate_sendfile, or sendfile_it_all), we check the apr_status_t and log
> > an error when rv != APR_SUCCESS.  However, apache only sets c->aborted if
> > the status code was ECONNABORTED, ECONNRESET, or EPIPE.
> >
> > Is there a reason we only set c->aborted for these specific
> > return values?
> > I think it may make more sense to always set c->aborted if we
> > encounter an
> > error.  This flag is the only way modules will know if something in the
> > filter chain went wrong.
> >
> > A patch is attached to always set c->aborted on network send errors.
> > (although I have only run into this problem with APR_TIMEUP errors, so
> >  we could also just add a APR_STATUS_IS_TIMEUP(rv) check)
> >
> > -Ryan
> >
> > Index: core.c
> > ===================================================================
> > RCS file: /home/cvspublic/httpd-2.0/server/core.c,v
> > retrieving revision 1.205
> > diff -u -r1.205 core.c
> > --- core.c      16 Sep 2002 13:11:55 -0000      1.205
> > +++ core.c      20 Sep 2002 20:46:52 -0000
> > @@ -3910,11 +3910,7 @@
> >              if (more)
> >                  apr_brigade_destroy(more);
> >
> > -            if (APR_STATUS_IS_ECONNABORTED(rv)
> > -                || APR_STATUS_IS_ECONNRESET(rv)
> > -                || APR_STATUS_IS_EPIPE(rv)) {
> > -                c->aborted = 1;
> > -            }
> > +            c->aborted = 1;
> >
> >              /* The client has aborted, but the request was successful. We
> >               * will report success, and leave it to the access and error
> >
> 
> 

Mime
View raw message