Skip to content

Commit

Permalink
new version3 of the stream stall patch, flushing meta buckets always
Browse files Browse the repository at this point in the history
  • Loading branch information
Stefan Eissing committed Jul 26, 2017
1 parent b0f913c commit 4e3730e
Showing 1 changed file with 225 additions and 0 deletions.
225 changes: 225 additions & 0 deletions patches/h2_stream_stall_2.4.x-v3.diff
View file
Original file line number Diff line number Diff line change
@@ -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 */

0 comments on commit 4e3730e

Please sign in to comment.