Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
Date: Thu, 13 Apr 2000 16:35:23 -0700 (PDT)
From: dean gaudet <dgaudet-list-new-httpd@arctic.org>
To: new-httpd@apache.org
Subject: Re: ap_send_fb_length() oddness
In-Reply-To: <069d01bfa592$170f4c30$c1e01b09@raleigh.ibm.com>
Message-ID: <Pine.LNX.4.21.0004131628270.13442-100000@twinlark.arctic.org>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII

On Thu, 13 Apr 2000, Bill Stoddard wrote:

> In fact, both of the places where we set the connection aborted look wrong to me. In both
> places, we are 'declaring' the network connection aborted because of return status from a
> read from a pipe.

this code doesn't look like it's correct for the ap_bread semantics in
2.0.  i can't even convince myself it was right using manoj's semantics
prior to my recent fix to chunking in buff... manoj -- even with your
semantics, wasn't it possible for APR_EOF to be returned in addition
to bytes being returned?

>   while (!ap_is_aborted(r->connection)) {
>         rv = ap_bread(fb, buf, sizeof(buf), &n);
>         if (n == 0) {
>             if (rv == APR_SUCCESS) {    /* eof */
>                 (void) ap_rflush(r);
>                 break;
>             }
>             if (rv != APR_EAGAIN) {
>                 r->connection->aborted = 1;   /* HERE */
>                 break;
>             }
>             /* next read will block, so flush the client now */
>             if (ap_rflush(r) == EOF) {
>                 break;
>             }
> 
>             ap_bsetopt(fb, BO_TIMEOUT, &r->server->timeout);
>             rv = ap_bread(fb, buf, sizeof(buf), &n);
>             if (n == 0) {
>                 if (rv == APR_SUCCESS) {        /* eof */
>                     (void) ap_rflush(r);
>                 }
>                 r->connection->aborted = 1;  /* AND HERE */
>                 break;
>             }
>             ap_bsetopt(fb, BO_TIMEOUT, &zero_timeout);
>         }

it should be more like this (including more of send_fb here):

    ap_status_t read_rv;

    ap_bsetopt(fb, BO_TIMEOUT, &zero_timeout);
    while (!ap_is_aborted(r->connection)) {
        read_rv = ap_bread(fb, buf, sizeof(buf), &n);

got_read:
	/* regardless of read error/eof/etc. there may have been bytes read */
        o = 0;
        while (n && !ap_is_aborted(r->connection)) {
            rv = ap_bwrite(r->connection->client, &buf[o], n, &w);
            if (w > 0) {
                total_bytes_sent += w;
                n -= w;
                o += w;
            }
            else if (rv != APR_SUCCESS) {
                if (!ap_is_aborted(r->connection)) {
                    ap_log_rerror(APLOG_MARK, APLOG_INFO, rv, r,
                        "client stopped connection before rflush completed");
                    ap_bsetflag(r->connection->client, B_EOUT, 1);
                    r->connection->aborted = 1;
                }
                break;
            }
        }

	if (read_rv == APR_SUCCESS) {
	    /* nada */
	}
	else if (read_rv == APR_EOF) {
	    (void) ap_rflush(r);
	    break;
	}
	else if (read_rv != APR_EAGAIN) {
	    r->connection->aborted = 1;
	    break;
	}
	else {
            /* next read will block, so flush the client now */
            if (ap_rflush(r) == EOF) {
                break;
            }

            ap_bsetopt(fb, BO_TIMEOUT, &r->server->timeout);
            read_rv = ap_bread(fb, buf, sizeof(buf), &n);
            ap_bsetopt(fb, BO_TIMEOUT, &zero_timeout);
	    goto got_read;
        }
    }
    SET_BYTES_SENT(r);