From b9389a2c361db9de759bc33dd8cdb979e778283f Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Wed, 4 Jan 2017 17:00:24 +0000 Subject: [PATCH] On the 2.4.x branch: merge of r1776738,1777160,1777324 from trunk backport of latest mod_http2 related changes. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1777344 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 6 +++ docs/manual/mod/mod_http2.xml | 4 +- modules/http2/h2_conn_io.c | 80 +++++++++++++++++++++++------------ modules/http2/h2_conn_io.h | 6 ++- modules/http2/h2_session.c | 54 +++++++++++++++++------ modules/http2/h2_session.h | 4 +- modules/http2/h2_stream.c | 7 +-- modules/http2/h2_version.h | 4 +- 8 files changed, 115 insertions(+), 50 deletions(-) diff --git a/CHANGES b/CHANGES index 9e1c40ac13..8fc388bccc 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,12 @@ Changes with Apache 2.4.26 + *) mod_http2: fix for possible page fault when stream is resumed during + session shutdown. [sidney-j-r-m (github)] + + *) mod_http2: fix for h2 session ignoring new responses while already + open streams continue to have data available. [Stefan Eissing] + *) mod_http2: adding support for MergeTrailers directive. [Stefan Eissing] *) mod_http2: limiting DATA frame sizes by TLS record sizes in use on the diff --git a/docs/manual/mod/mod_http2.xml b/docs/manual/mod/mod_http2.xml index b7dcc959be..63bcf9f74b 100644 --- a/docs/manual/mod/mod_http2.xml +++ b/docs/manual/mod/mod_http2.xml @@ -268,9 +268,7 @@ H2Direct on

This directive toggles the usage of the HTTP/2 server push - protocol feature. This should be used inside a - VirtualHost - section to enable direct HTTP/2 communication for that virtual host. + protocol feature.

The HTTP/2 protocol allows the server to push other resources to diff --git a/modules/http2/h2_conn_io.c b/modules/http2/h2_conn_io.c index 13d29913df..f3126bb71f 100644 --- a/modules/http2/h2_conn_io.c +++ b/modules/http2/h2_conn_io.c @@ -128,13 +128,14 @@ static void h2_conn_io_bb_log(conn_rec *c, int stream_id, int level, apr_status_t h2_conn_io_init(h2_conn_io *io, conn_rec *c, const h2_config *cfg) { - io->c = c; - io->output = apr_brigade_create(c->pool, c->bucket_alloc); - io->is_tls = h2_h2_is_tls(c); - io->buffer_output = io->is_tls; - io->pass_threshold = (apr_size_t)h2_config_geti64(cfg, H2_CONF_STREAM_MAX_MEM) / 2; - io->flush_factor = h2_config_geti(cfg, H2_CONF_TLS_FLUSH_COUNT); - + io->c = c; + io->output = apr_brigade_create(c->pool, c->bucket_alloc); + io->is_tls = h2_h2_is_tls(c); + io->buffer_output = io->is_tls; + io->pass_threshold = (apr_size_t)h2_config_geti64(cfg, H2_CONF_STREAM_MAX_MEM); + io->flush_factor = h2_config_geti(cfg, H2_CONF_TLS_FLUSH_COUNT); + io->speed_factor = 1.0; + if (io->is_tls) { /* This is what we start with, * see https://issues.apache.org/jira/browse/TS-2503 @@ -256,6 +257,7 @@ static void check_write_size(h2_conn_io *io) /* long time not written, reset write size */ io->write_size = WRITE_SIZE_INITIAL; io->bytes_written = 0; + io->speed_factor = 1.0; ap_log_cerror(APLOG_MARK, APLOG_TRACE4, 0, io->c, "h2_conn_io(%ld): timeout write size reset to %ld", (long)io->c->id, (long)io->write_size); @@ -325,7 +327,34 @@ static apr_status_t pass_output(h2_conn_io *io, int flush, apr_status_t h2_conn_io_flush(h2_conn_io *io) { - return pass_output(io, 1, NULL); + apr_time_t start = 0; + apr_status_t status; + + if (io->needs_flush > 0) { + /* this is a buffer size triggered flush, let's measure how + * long it takes and try to adjust our speed factor accordingly */ + start = apr_time_now(); + } + status = pass_output(io, 1, NULL); + check_write_size(io); + if (start && status == APR_SUCCESS) { + apr_time_t duration = apr_time_now() - start; + if (duration < apr_time_from_msec(100)) { + io->speed_factor *= 1.0 + ((apr_time_from_msec(100) - duration) + / (float)apr_time_from_msec(100)); + ap_log_cerror(APLOG_MARK, APLOG_TRACE3, status, io->c, + "h2_conn_io(%ld): incr speed_factor to %f", + io->c->id, io->speed_factor); + } + else if (duration > apr_time_from_msec(200)) { + io->speed_factor *= 0.5; + ap_log_cerror(APLOG_MARK, APLOG_TRACE3, status, io->c, + "h2_conn_io(%ld): decr speed_factor to %f", + io->c->id, io->speed_factor); + } + } + io->needs_flush = -1; + return status; } apr_status_t h2_conn_io_write_eoc(h2_conn_io *io, h2_session *session) @@ -367,6 +396,7 @@ apr_status_t h2_conn_io_write(h2_conn_io *io, const char *data, size_t length) else { status = apr_brigade_write(io->output, NULL, NULL, data, length); } + io->needs_flush = -1; return status; } @@ -375,7 +405,6 @@ apr_status_t h2_conn_io_pass(h2_conn_io *io, apr_bucket_brigade *bb) apr_bucket *b; apr_status_t status = APR_SUCCESS; - check_write_size(io); while (!APR_BRIGADE_EMPTY(bb) && status == APR_SUCCESS) { b = APR_BRIGADE_FIRST(bb); @@ -418,24 +447,21 @@ apr_status_t h2_conn_io_pass(h2_conn_io *io, apr_bucket_brigade *bb) APR_BRIGADE_INSERT_TAIL(io->output, b); } } - - if (status == APR_SUCCESS) { - if (!APR_BRIGADE_EMPTY(io->output)) { - apr_off_t len; - if (io->buffer_output) { - apr_brigade_length(io->output, 0, &len); - if (len >= (io->flush_factor * io->write_size)) { - return pass_output(io, 1, NULL); - } - } - else { - len = h2_brigade_mem_size(io->output); - if (len >= io->pass_threshold) { - return pass_output(io, 0, NULL); - } - } - } - } + io->needs_flush = -1; return status; } +int h2_conn_io_needs_flush(h2_conn_io *io) +{ + if (io->needs_flush < 0) { + apr_off_t len; + apr_brigade_length(io->output, 0, &len); + if (len > (io->pass_threshold * io->flush_factor * io->speed_factor)) { + /* don't want to keep too much around */ + io->needs_flush = 1; + return 1; + } + io->needs_flush = 0; + } + return 0; +} diff --git a/modules/http2/h2_conn_io.h b/modules/http2/h2_conn_io.h index bda0c8ff6d..f617018565 100644 --- a/modules/http2/h2_conn_io.h +++ b/modules/http2/h2_conn_io.h @@ -39,8 +39,10 @@ typedef struct { apr_int64_t bytes_written; int buffer_output; + int needs_flush; apr_size_t pass_threshold; - int flush_factor; + float flush_factor; + float speed_factor; char *scratch; apr_size_t ssize; @@ -74,4 +76,6 @@ apr_status_t h2_conn_io_write_eoc(h2_conn_io *io, struct h2_session *session); */ apr_status_t h2_conn_io_flush(h2_conn_io *io); +int h2_conn_io_needs_flush(h2_conn_io *io); + #endif /* defined(__mod_h2__h2_conn_io__) */ diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c index f2db2997f7..ad313f3bdd 100644 --- a/modules/http2/h2_session.c +++ b/modules/http2/h2_session.c @@ -47,6 +47,8 @@ #include "h2_workers.h" +static apr_status_t dispatch_master(h2_session *session); + static int h2_session_status_from_apr_status(apr_status_t rv) { if (rv == APR_SUCCESS) { @@ -238,6 +240,17 @@ static ssize_t send_cb(nghttp2_session *ngh2, (void)ngh2; (void)flags; + if (h2_conn_io_needs_flush(&session->io)) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, session->c, + "h2_session(%ld): blocking due to io flush", + session->id); + status = h2_conn_io_flush(&session->io); + if (status != APR_SUCCESS) { + return h2_session_status_from_apr_status(status); + } + return NGHTTP2_ERR_WOULDBLOCK; + } + status = h2_conn_io_write(&session->io, (const char *)data, length); if (status == APR_SUCCESS) { return length; @@ -576,6 +589,17 @@ static int on_send_data_cb(nghttp2_session *ngh2, (void)ngh2; (void)source; + if (h2_conn_io_needs_flush(&session->io)) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, session->c, + "h2_stream(%ld-%d): blocking due to io flush", + session->id, stream_id); + status = h2_conn_io_flush(&session->io); + if (status != APR_SUCCESS) { + return h2_session_status_from_apr_status(status); + } + return NGHTTP2_ERR_WOULDBLOCK; + } + if (frame->data.padlen > H2_MAX_PADLEN) { return NGHTTP2_ERR_PROTO; } @@ -1402,7 +1426,7 @@ static apr_status_t h2_session_send(h2_session *session) apr_socket_timeout_set(socket, saved_timeout); } session->have_written = 1; - if (rv != 0) { + if (rv != 0 && rv != NGHTTP2_ERR_WOULDBLOCK) { if (nghttp2_is_fatal(rv)) { dispatch_event(session, H2_SESSION_EV_PROTO_ERROR, rv, nghttp2_strerror(rv)); return APR_EGENERAL; @@ -2028,6 +2052,21 @@ static void dispatch_event(h2_session *session, h2_session_event_t ev, } } +/* trigger window updates, stream resumes and submits */ +static apr_status_t dispatch_master(h2_session *session) { + apr_status_t status; + + status = h2_mplx_dispatch_master_events(session->mplx, + on_stream_resume, session); + if (status != APR_SUCCESS) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE3, status, session->c, + "h2_session(%ld): dispatch error", session->id); + dispatch_event(session, H2_SESSION_EV_CONN_ERROR, + H2_ERR_INTERNAL_ERROR, "dispatch error"); + } + return status; +} + static const int MAX_WAIT_MICROS = 200 * 1000; apr_status_t h2_session_process(h2_session *session, int async) @@ -2203,18 +2242,9 @@ apr_status_t h2_session_process(h2_session *session, int async) dispatch_event(session, H2_SESSION_EV_CONN_ERROR, 0, NULL); } } - - /* trigger window updates, stream resumes and submits */ - status = h2_mplx_dispatch_master_events(session->mplx, - on_stream_resume, - session); + + status = dispatch_master(session); if (status != APR_SUCCESS) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE3, status, c, - "h2_session(%ld): dispatch error", - session->id); - dispatch_event(session, H2_SESSION_EV_CONN_ERROR, - H2_ERR_INTERNAL_ERROR, - "dispatch error"); break; } diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h index 69b7a59c44..417e3c1ebb 100644 --- a/modules/http2/h2_session.h +++ b/modules/http2/h2_session.h @@ -102,9 +102,9 @@ typedef struct h2_session { struct h2_push_diary *push_diary; /* remember pushes, avoid duplicates */ - int open_streams; /* number of streams open */ + int open_streams; /* number of client streams open */ int unsent_submits; /* number of submitted, but not yet written responses. */ - int unsent_promises; /* number of submitted, but not yet written push promised */ + int unsent_promises; /* number of submitted, but not yet written push promises */ int responses_submitted; /* number of http/2 responses submitted */ int streams_reset; /* number of http/2 streams reset by client */ diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c index 18bcf55fd6..fcdaa4ddc3 100644 --- a/modules/http2/h2_stream.c +++ b/modules/http2/h2_stream.c @@ -600,10 +600,10 @@ static apr_bucket *get_first_headers_bucket(apr_bucket_brigade *bb) apr_status_t h2_stream_out_prepare(h2_stream *stream, apr_off_t *plen, int *peos, h2_headers **presponse) { - conn_rec *c = stream->session->c; apr_status_t status = APR_SUCCESS; apr_off_t requested, max_chunk = H2_DATA_CHUNK_SIZE; apr_bucket *b, *e; + conn_rec *c; if (presponse) { *presponse = NULL; @@ -614,10 +614,11 @@ apr_status_t h2_stream_out_prepare(h2_stream *stream, apr_off_t *plen, *peos = 1; return APR_ECONNRESET; } - - if (!output_open(stream)) { + else if (!output_open(stream)) { return APR_ECONNRESET; } + + c = stream->session->c; prep_output(stream); if (stream->session->io.write_size > 0) { diff --git a/modules/http2/h2_version.h b/modules/http2/h2_version.h index a98fee57ce..3a5e64e693 100644 --- a/modules/http2/h2_version.h +++ b/modules/http2/h2_version.h @@ -26,7 +26,7 @@ * @macro * Version number of the http2 module as c string */ -#define MOD_HTTP2_VERSION "1.8.6" +#define MOD_HTTP2_VERSION "1.8.7" /** * @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 0x010806 +#define MOD_HTTP2_VERSION_NUM 0x010807 #endif /* mod_h2_h2_version_h */ -- 2.40.0