httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jus...@erenkrantz.com>
Subject Re: c->aborted is not set correctly on trunk
Date Thu, 05 Jan 2006 16:14:58 GMT
On Thu, Jan 05, 2006 at 05:02:54PM +0100, Ruediger Pluem wrote:
> 
> 
> On 01/02/2006 04:14 PM, Ruediger Pluem wrote:
> > I just noticed that c->aborted does not get set on trunk when you abort a connection
in the browser (with worker mpm,
> > but I guess with prefork is the same).
> > This works in 2.2.x. I guess this has something to do with the async write refactoring
in the trunk compared
> > to 2.2.x. In 2.2.x c->aborted gets set in the core output filter whereas in the
trunk this has vanished.
> 
> I had a look into this and created a patch that solved my test case. But as the core
output filter is a fundamental
> part of the code I would like to have some remote eyes on it. My approach was to simulate
the 'old behaviour'
> in 2.2.x:
> 
> 
> Index: server/core_filters.c
> ===================================================================
> --- server/core_filters.c(Revision 366181)
> +++ server/core_filters.c(Arbeitskopie)
> @@ -413,11 +413,12 @@
>      if (new_bb == NULL) {
>          apr_status_t rv = send_brigade_nonblocking(net->client_socket, bb,
>                                                     &(ctx->bytes_written), c);
> -        if (APR_STATUS_IS_EAGAIN(rv)) {
> -            rv = APR_SUCCESS;
> +        if ((rv != APR_SUCCESS) && !APR_STATUS_IS_EAGAIN(rv)) {
> +            /* The client aborted the connection */
> +            c->aborted = 1;
>          }
>          setaside_remaining_output(f, ctx, bb, 0, c);
> -        return rv;
> +        return APR_SUCCESS;
>      }

Why are we masking the non-EAGAIN error values too?  I'd prefer that we
continue to return the error code for everything but EAGAIN - like the
current code does.  (Setting c->aborted here probably does make sense
though.)

I do note that the 2.0 code says:

            /* The client has aborted, but the request was successful. We
             * will report success, and leave it to the access and error
             * logs to note that the connection was aborted.
             */
            return APR_SUCCESS;

I'm just not sure I agree with that.  -- justin

Mime
View raw message