diff --git a/ChangeLog b/ChangeLog index edfef7a6..070b4368 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +v1.10.8 +-------------------------------------------------------------------------------- + * a stream could stall and not send any more output to the client which in the end aborts the connection due to a timeout. + v1.10.7 -------------------------------------------------------------------------------- * disable and give warning when mpm_prefork is encountered. The server will diff --git a/configure.ac b/configure.ac index e8593fc7..ea0a2d78 100644 --- a/configure.ac +++ b/configure.ac @@ -14,7 +14,7 @@ # AC_PREREQ([2.69]) -AC_INIT([mod_http2], [1.10.7], [stefan.eissing@greenbytes.de]) +AC_INIT([mod_http2], [1.10.8], [stefan.eissing@greenbytes.de]) LT_PREREQ([2.2.6]) LT_INIT() diff --git a/mod_http2/h2_stream.c b/mod_http2/h2_stream.c index 925f3d6d..1e87b1cb 100644 --- a/mod_http2/h2_stream.c +++ b/mod_http2/h2_stream.c @@ -774,20 +774,20 @@ static apr_bucket *get_first_headers_bucket(apr_bucket_brigade *bb) return NULL; } -static apr_status_t add_data(h2_stream *stream, apr_off_t requested, - apr_off_t *plen, int *peos, int *complete, - h2_headers **pheaders) +static apr_status_t add_buffered_data(h2_stream *stream, apr_off_t requested, + apr_off_t *plen, int *peos, int *is_all, + h2_headers **pheaders) { apr_bucket *b, *e; *peos = 0; *plen = 0; - *complete = 0; + *is_all = 0; if (pheaders) { *pheaders = NULL; } - H2_STREAM_OUT_LOG(APLOG_TRACE2, stream, "add_data"); + H2_STREAM_OUT_LOG(APLOG_TRACE2, stream, "add_buffered_data"); b = APR_BRIGADE_FIRST(stream->out_buffer); while (b != APR_BRIGADE_SENTINEL(stream->out_buffer)) { e = APR_BUCKET_NEXT(b); @@ -833,7 +833,7 @@ static apr_status_t add_data(h2_stream *stream, apr_off_t requested, } b = e; } - *complete = 1; + *is_all = 1; return APR_SUCCESS; } @@ -865,7 +865,7 @@ apr_status_t h2_stream_out_prepare(h2_stream *stream, apr_off_t *plen, requested = (*plen > 0)? H2MIN(*plen, max_chunk) : max_chunk; /* count the buffered data until eos or a headers bucket */ - status = add_data(stream, requested, plen, peos, &complete, pheaders); + status = add_buffered_data(stream, requested, plen, peos, &complete, pheaders); if (status == APR_EAGAIN) { /* TODO: ugly, someone needs to retrieve the response first */ @@ -882,28 +882,38 @@ apr_status_t h2_stream_out_prepare(h2_stream *stream, apr_off_t *plen, return APR_SUCCESS; } + /* If there we do not have enough buffered data to satisfy the requested + * length *and* we counted the _complete_ buffer (and did not stop in the middle + * because of meta data there), lets see if we can read more from the + * output beam */ missing = H2MIN(requested, stream->max_mem) - *plen; if (complete && !*peos && missing > 0) { + apr_status_t rv = APR_EOF; + if (stream->output) { H2_STREAM_OUT_LOG(APLOG_TRACE2, stream, "pre"); - status = h2_beam_receive(stream->output, stream->out_buffer, - APR_NONBLOCK_READ, - stream->max_mem - *plen); + rv = h2_beam_receive(stream->output, stream->out_buffer, + APR_NONBLOCK_READ, stream->max_mem - *plen); H2_STREAM_OUT_LOG(APLOG_TRACE2, stream, "post"); } - else { - status = APR_EOF; - } - if (APR_STATUS_IS_EOF(status)) { + if (rv == APR_SUCCESS) { + /* count the buffer again, now that we have read output */ + status = add_buffered_data(stream, requested, plen, peos, &complete, pheaders); + } + else if (APR_STATUS_IS_EOF(rv)) { apr_bucket *eos = apr_bucket_eos_create(c->bucket_alloc); APR_BRIGADE_INSERT_TAIL(stream->out_buffer, eos); *peos = 1; - status = APR_SUCCESS; } - else if (status == APR_SUCCESS) { - /* do it again, now that we have gotten more */ - status = add_data(stream, requested, plen, peos, &complete, pheaders); + else if (APR_STATUS_IS_EAGAIN(rv)) { + /* we set this is the status of this call only if there + * is no buffered data, see check below */ + } + else { + /* real error reading. Give this back directly, even though + * we may have something buffered. */ + status = rv; } } diff --git a/mod_http2/h2_task.c b/mod_http2/h2_task.c index 1ef0d9a8..2ab41455 100644 --- a/mod_http2/h2_task.c +++ b/mod_http2/h2_task.c @@ -129,7 +129,7 @@ static apr_status_t slave_out(h2_task *task, ap_filter_t* f, apr_bucket_brigade* bb) { apr_bucket *b; - apr_status_t status = APR_SUCCESS; + apr_status_t rv = APR_SUCCESS; int flush = 0, blocking; if (task->frozen) { @@ -148,17 +148,16 @@ static apr_status_t slave_out(h2_task *task, ap_filter_t* f, return APR_SUCCESS; } +send: /* we send block once we opened the output, so someone is there * reading it *and* the task is not assigned to a h2_req_engine */ blocking = (!task->assigned && task->output.opened); - if (!task->output.opened) { - for (b = APR_BRIGADE_FIRST(bb); - b != APR_BRIGADE_SENTINEL(bb); - b = APR_BUCKET_NEXT(b)) { - if (APR_BUCKET_IS_FLUSH(b)) { - flush = 1; - break; - } + for (b = APR_BRIGADE_FIRST(bb); + b != APR_BRIGADE_SENTINEL(bb); + b = APR_BUCKET_NEXT(b)) { + if (APR_BUCKET_IS_FLUSH(b) || APR_BUCKET_IS_EOS(b) || AP_BUCKET_IS_EOR(b)) { + flush = 1; + break; } } @@ -166,32 +165,48 @@ static apr_status_t slave_out(h2_task *task, ap_filter_t* f, /* still have data buffered from previous attempt. * setaside and append new data and try to pass the complete data */ if (!APR_BRIGADE_EMPTY(bb)) { - status = ap_save_brigade(f, &task->output.bb, &bb, task->pool); + if (APR_SUCCESS != (rv = ap_save_brigade(f, &task->output.bb, &bb, task->pool))) { + goto out; + } } - if (status == APR_SUCCESS) { - status = send_out(task, task->output.bb, blocking); - } + rv = send_out(task, task->output.bb, blocking); } else { - /* no data buffered here, try to pass the brigade directly */ - status = send_out(task, bb, blocking); - if (status == APR_SUCCESS && !APR_BRIGADE_EMPTY(bb)) { - /* could not write all, buffer the rest */ - ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, task->c, APLOGNO(03405) - "h2_slave_out(%s): saving brigade", - task->id); - status = ap_save_brigade(f, &task->output.bb, &bb, task->pool); - flush = 1; + /* no data buffered previously, pass brigade directly */ + rv = send_out(task, bb, blocking); + + if (APR_SUCCESS == rv && !APR_BRIGADE_EMPTY(bb)) { + /* output refused to buffer it all, time to open? */ + if (!task->output.opened && APR_SUCCESS == (rv = open_output(task))) { + /* Make another attempt to send the data. With the output open, + * the call might be blocking and send all data, so we do not need + * to save the brigade */ + goto send; + } + else if (blocking && flush) { + /* Need to keep on doing this. */ + goto send; + } + + if (APR_SUCCESS == rv) { + /* could not write all, buffer the rest */ + ap_log_cerror(APLOG_MARK, APLOG_DEBUG, rv, task->c, APLOGNO(03405) + "h2_slave_out(%s): saving brigade", task->id); + ap_assert(NULL); + rv = ap_save_brigade(f, &task->output.bb, &bb, task->pool); + flush = 1; + } } } - if (status == APR_SUCCESS && !task->output.opened && flush) { + if (APR_SUCCESS == rv && !task->output.opened && flush) { /* got a flush or could not write all, time to tell someone to read */ - status = open_output(task); + rv = open_output(task); } - ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, task->c, +out: + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, rv, task->c, "h2_slave_out(%s): slave_out leave", task->id); - return status; + return rv; } static apr_status_t output_finish(h2_task *task) diff --git a/mod_http2/h2_version.h b/mod_http2/h2_version.h index e3affa27..19ab9508 100644 --- a/mod_http2/h2_version.h +++ b/mod_http2/h2_version.h @@ -26,7 +26,7 @@ * @macro * Version number of the http2 module as c string */ -#define MOD_HTTP2_VERSION "1.10.7-git" +#define MOD_HTTP2_VERSION "1.10.8-git" /** * @macro @@ -34,7 +34,7 @@ * release. This is a 24 bit number with 8 bits for major number, 8 bits * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203. */ -#define MOD_HTTP2_VERSION_NUM 0x010a07 +#define MOD_HTTP2_VERSION_NUM 0x010a08 #endif /* mod_h2_h2_version_h */