From 4e3730ee9c772bdda48cafcab049d3fe9073d0dd Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Wed, 26 Jul 2017 16:17:11 +0200 Subject: [PATCH] new version3 of the stream stall patch, flushing meta buckets always --- patches/h2_stream_stall_2.4.x-v3.diff | 225 ++++++++++++++++++++++++++ 1 file changed, 225 insertions(+) create mode 100644 patches/h2_stream_stall_2.4.x-v3.diff diff --git a/patches/h2_stream_stall_2.4.x-v3.diff b/patches/h2_stream_stall_2.4.x-v3.diff new file mode 100644 index 00000000..9b8b62e6 --- /dev/null +++ b/patches/h2_stream_stall_2.4.x-v3.diff @@ -0,0 +1,225 @@ +Index: modules/http2/h2_stream.c +=================================================================== +--- modules/http2/h2_stream.c (revision 1802906) ++++ modules/http2/h2_stream.c (working copy) +@@ -774,20 +774,20 @@ + 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 @@ + } + b = e; + } +- *complete = 1; ++ *is_all = 1; + return APR_SUCCESS; + } + +@@ -865,7 +865,7 @@ + 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,29 +882,39 @@ + 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 (rv == APR_SUCCESS) { ++ /* count the buffer again, now that we have read output */ ++ status = add_buffered_data(stream, requested, plen, peos, &complete, pheaders); + } +- +- if (APR_STATUS_IS_EOF(status)) { ++ 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; ++ } + } + + if (status == APR_SUCCESS) { +Index: modules/http2/h2_task.c +=================================================================== +--- modules/http2/h2_task.c (revision 1802906) ++++ modules/http2/h2_task.c (working copy) +@@ -129,7 +129,7 @@ + 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 @@ + 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 @@ + /* 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) +Index: modules/http2/h2_version.h +=================================================================== +--- modules/http2/h2_version.h (revision 1802906) ++++ modules/http2/h2_version.h (working copy) +@@ -26,7 +26,7 @@ + * @macro + * Version number of the http2 module as c string + */ +-#define MOD_HTTP2_VERSION "1.10.7" ++#define MOD_HTTP2_VERSION "1.10.8" + + /** + * @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 0x010a06 ++#define MOD_HTTP2_VERSION_NUM 0x010a08 + + + #endif /* mod_h2_h2_version_h */