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
