From: Stefan Eissing Date: Fri, 18 Mar 2016 14:48:36 +0000 (+0000) Subject: Merge of 1735608,1735609 from trunk: X-Git-Tag: 2.4.19~48 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0116d13679191612b9d6a31cbb8cb77597bb7446;p=apache Merge of 1735608,1735609 from trunk: mod_http2: stream cleanup on GOAWAY handling, PUSHes prohibited after client GOAWAY. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1735610 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index 5263cafa5a..3a003371a7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,10 +1,7 @@ -*- coding: utf-8 -*- -Changes with Apache 2.4.19 - - *) mod_http2: slave connections are reused for several requests, improved - performance and better memory use. [Stefan Eissing] - + *) mod_http2: disabling PUSH when client sends GOAWAY. + *) mod_rewrite: Don't implicitly URL-escape the original query string when no substitution has changed it (like PR50447 but server context) [Evgeny Kotkov ] diff --git a/modules/http2/h2.h b/modules/http2/h2.h index 89d174e237..335d368899 100644 --- a/modules/http2/h2.h +++ b/modules/http2/h2.h @@ -99,6 +99,16 @@ typedef enum { H2_SESSION_ST_REMOTE_SHUTDOWN, /* client announced GOAWAY */ } h2_session_state; +typedef struct h2_session_props { + apr_uint32_t accepted_max; /* the highest remote stream id was/will be handled */ + apr_uint32_t completed_max; /* the highest remote stream completed */ + apr_uint32_t emitted_count; /* the number of local streams sent */ + apr_uint32_t emitted_max; /* the highest local stream id sent */ + apr_uint32_t error; /* the last session error encountered */ + unsigned int accepting : 1; /* if the session is accepting new streams */ +} h2_session_props; + + /* h2_request is the transformer of HTTP2 streams into HTTP/1.1 internal * format that will be fed to various httpd input filters to finally * become a request_rec to be handled by soemone. diff --git a/modules/http2/h2_conn_io.c b/modules/http2/h2_conn_io.c index 59561ecd61..2b7e769410 100644 --- a/modules/http2/h2_conn_io.c +++ b/modules/http2/h2_conn_io.c @@ -265,7 +265,7 @@ static apr_status_t h2_conn_io_flush_int(h2_conn_io *io, int flush, int eoc) pass_out_ctx ctx; apr_bucket *b; - if (io->buflen == 0 && APR_BRIGADE_EMPTY(io->output)) { + if (!flush && io->buflen == 0 && APR_BRIGADE_EMPTY(io->output)) { return APR_SUCCESS; } diff --git a/modules/http2/h2_filter.c b/modules/http2/h2_filter.c index 751480e73f..92e87bcd17 100644 --- a/modules/http2/h2_filter.c +++ b/modules/http2/h2_filter.c @@ -218,7 +218,7 @@ static apr_status_t h2_sos_h2_status_buffer(h2_sos *sos, apr_bucket_brigade *bb) bbout(" \"this_stream\": %d,\n", stream->id); bbout(" \"streams_open\": %d,\n", (int)h2_ihash_count(session->streams)); bbout(" \"max_stream_started\": %d,\n", mplx->max_stream_started); - bbout(" \"requests_received\": %d,\n", session->requests_received); + bbout(" \"requests_received\": %d,\n", session->remote.emitted_count); bbout(" \"responses_submitted\": %d,\n", session->responses_submitted); bbout(" \"streams_reset\": %d, \n", session->streams_reset); bbout(" \"pushes_promised\": %d,\n", session->pushes_promised); diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c index 1133807c33..5c2319ce94 100644 --- a/modules/http2/h2_mplx.c +++ b/modules/http2/h2_mplx.c @@ -227,23 +227,22 @@ h2_mplx *h2_mplx_create(conn_rec *c, apr_pool_t *parent, return m; } -int h2_mplx_get_max_stream_started(h2_mplx *m) +apr_uint32_t h2_mplx_shutdown(h2_mplx *m) { - int stream_id = 0; - int acquired; - - enter_mutex(m, &acquired); - stream_id = m->max_stream_started; - leave_mutex(m, acquired); + int acquired, max_stream_started = 0; - return stream_id; + if (enter_mutex(m, &acquired) == APR_SUCCESS) { + max_stream_started = m->max_stream_started; + /* Clear schedule queue, disabling existing streams from starting */ + h2_iq_clear(m->q); + leave_mutex(m, acquired); + } + return max_stream_started; } static void workers_register(h2_mplx *m) { - /* Initially, there was ref count increase for this as well, but - * this is not needed, even harmful. - * h2_workers is only a hub for all the h2_worker instances. + /* h2_workers is only a hub for all the h2_worker instances. * At the end-of-life of this h2_mplx, we always unregister at * the workers. The thing to manage are all the h2_worker instances * out there. Those may hold a reference to this h2_mplx and we cannot @@ -311,6 +310,10 @@ static void io_destroy(h2_mplx *m, h2_io *io, int events) } } + if (io->eor) { + apr_bucket_delete(io->eor); + io->eor = NULL; + } if (io->pool) { apr_pool_destroy(io->pool); } @@ -473,7 +476,6 @@ apr_status_t h2_mplx_stream_done(h2_mplx *m, int stream_id, int rst_error) m->id, stream_id); io_stream_done(m, io, rst_error); } - leave_mutex(m, acquired); } return status; @@ -992,7 +994,6 @@ apr_status_t h2_mplx_reprioritize(h2_mplx *m, h2_stream_pri_cmp *cmp, void *ctx) } else { h2_iq_sort(m->q, cmp, ctx); - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, m->c, "h2_mplx(%ld): reprioritize tasks", m->id); } diff --git a/modules/http2/h2_mplx.h b/modules/http2/h2_mplx.h index eef8bf1aba..dac5aac4c0 100644 --- a/modules/http2/h2_mplx.h +++ b/modules/http2/h2_mplx.h @@ -63,12 +63,12 @@ typedef void h2_mplx_consumed_cb(void *ctx, int stream_id, apr_off_t consumed); struct h2_mplx { long id; - APR_RING_ENTRY(h2_mplx) link; - volatile int refs; conn_rec *c; apr_pool_t *pool; apr_bucket_alloc_t *bucket_alloc; + APR_RING_ENTRY(h2_mplx) link; + unsigned int aborted : 1; unsigned int need_registration : 1; @@ -146,12 +146,11 @@ struct h2_task *h2_mplx_pop_task(h2_mplx *mplx, int *has_more); void h2_mplx_task_done(h2_mplx *m, struct h2_task *task, struct h2_task **ptask); /** - * Get the highest stream identifier that has been passed on to processing. - * Maybe 0 in case no stream has been processed yet. - * @param m the multiplexer - * @return highest stream identifier for which processing started + * Shut down the multiplexer gracefully. Will no longer schedule new streams + * but let the ongoing ones finish normally. + * @return the highest stream id being/been processed */ -int h2_mplx_get_max_stream_started(h2_mplx *m); +apr_uint32_t h2_mplx_shutdown(h2_mplx *m); /******************************************************************************* * IO lifetime of streams. diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c index 4372353c13..cbdae8cbbf 100644 --- a/modules/http2/h2_session.c +++ b/modules/http2/h2_session.c @@ -78,6 +78,49 @@ static int is_accepting_streams(h2_session *session); static void dispatch_event(h2_session *session, h2_session_event_t ev, int err, const char *msg); +typedef struct stream_sel_ctx { + h2_session *session; + h2_stream *candidate; +} stream_sel_ctx; + +static int find_cleanup_stream(void *udata, void *sdata) +{ + stream_sel_ctx *ctx = udata; + h2_stream *stream = sdata; + if (H2_STREAM_CLIENT_INITIATED(stream->id)) { + if (!ctx->session->local.accepting + && stream->id > ctx->session->local.accepted_max) { + ctx->candidate = stream; + return 0; + } + } + else { + if (!ctx->session->remote.accepting + && stream->id > ctx->session->remote.accepted_max) { + ctx->candidate = stream; + return 0; + } + } + return 1; +} + +static void cleanup_streams(h2_session *session) +{ + stream_sel_ctx ctx; + ctx.session = session; + ctx.candidate = NULL; + while (1) { + h2_ihash_iter(session->streams, find_cleanup_stream, &ctx); + if (ctx.candidate) { + h2_session_stream_destroy(session, ctx.candidate); + ctx.candidate = NULL; + } + else { + break; + } + } +} + h2_stream *h2_session_open_stream(h2_session *session, int stream_id) { h2_stream * stream; @@ -95,10 +138,18 @@ h2_stream *h2_session_open_stream(h2_session *session, int stream_id) stream = h2_stream_open(stream_id, stream_pool, session); h2_ihash_add(session->streams, stream); - if (H2_STREAM_CLIENT_INITIATED(stream_id) - && stream_id > session->max_stream_received) { - ++session->requests_received; - session->max_stream_received = stream->id; + if (H2_STREAM_CLIENT_INITIATED(stream_id)) { + if (stream_id > session->remote.emitted_max) { + ++session->remote.emitted_count; + session->remote.emitted_max = stream->id; + session->local.accepted_max = stream->id; + } + } + else { + if (stream_id > session->local.emitted_max) { + ++session->local.emitted_count; + session->remote.emitted_max = stream->id; + } } return stream; @@ -262,8 +313,10 @@ static apr_status_t stream_release(h2_session *session, ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c, "h2_stream(%ld-%d): handled, closing", session->id, (int)stream->id); - if (stream->id > session->max_stream_handled) { - session->max_stream_handled = stream->id; + if (H2_STREAM_CLIENT_INITIATED(stream->id)) { + if (stream->id > session->local.completed_max) { + session->local.completed_max = stream->id; + } } } else { @@ -309,7 +362,7 @@ static int on_begin_headers_cb(nghttp2_session *ngh2, else { s = h2_session_open_stream((h2_session *)userp, frame->hd.stream_id); } - return s? 0 : NGHTTP2_ERR_CALLBACK_FAILURE; + return s? 0 : NGHTTP2_ERR_START_STREAM_NOT_ALLOWED; } static int on_header_cb(nghttp2_session *ngh2, const nghttp2_frame *frame, @@ -446,6 +499,8 @@ static int on_frame_recv_cb(nghttp2_session *ng2s, } break; case NGHTTP2_GOAWAY: + session->remote.accepted_max = frame->goaway.last_stream_id; + session->remote.error = frame->goaway.error_code; dispatch_event(session, H2_SESSION_EV_REMOTE_GOAWAY, 0, NULL); break; default: @@ -672,27 +727,43 @@ static void h2_session_destroy(h2_session *session) } } -static apr_status_t h2_session_shutdown(h2_session *session, int reason, +static apr_status_t h2_session_shutdown(h2_session *session, int error, const char *msg, int force_close) { apr_status_t status = APR_SUCCESS; - const char *err = msg; AP_DEBUG_ASSERT(session); - if (!err && reason) { - err = nghttp2_strerror(reason); + if (!msg && error) { + msg = nghttp2_strerror(error); + } + + if (error || force_close) { + /* not a graceful shutdown, we want to leave... + * Do not start further streams that are waiting to be scheduled. + * Find out the max stream id that we habe been processed or + * are still actively working on. + * Remove all streams greater than this number without submitting + * a RST_STREAM frame, since that should be clear from the GOAWAY + * we send. */ + session->local.accepted_max = h2_mplx_shutdown(session->mplx); + session->local.error = error; + } + else { + /* graceful shutdown. we will continue processing all streams + * we have, but no longer accept new ones. Report the max stream + * we have received and discard all new ones. */ } nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE, - h2_mplx_get_max_stream_started(session->mplx), - reason, (uint8_t*)err, err? strlen(err):0); + session->local.accepted_max, + error, (uint8_t*)msg, msg? strlen(msg):0); status = nghttp2_session_send(session->ngh2); if (status == APR_SUCCESS) { status = h2_conn_io_flush(&session->io); } ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c, APLOGNO(03069) "session(%ld): sent GOAWAY, err=%d, msg=%s", - session->id, reason, err? err : ""); - dispatch_event(session, H2_SESSION_EV_LOCAL_GOAWAY, reason, err); + session->id, error, msg? msg : ""); + dispatch_event(session, H2_SESSION_EV_LOCAL_GOAWAY, error, msg); if (force_close) { h2_mplx_abort(session->mplx); @@ -799,30 +870,36 @@ static h2_session *h2_session_create_int(conn_rec *c, session->c = c; session->r = r; session->s = h2_ctx_server_get(ctx); + session->pool = pool; session->config = h2_config_sget(session->s); + session->workers = workers; session->state = H2_SESSION_ST_INIT; + session->local.accepting = 1; + session->remote.accepting = 1; - session->pool = pool; apr_pool_pre_cleanup_register(pool, session, session_pool_cleanup); - session->max_stream_count = h2_config_geti(session->config, H2_CONF_MAX_STREAMS); - session->max_stream_mem = h2_config_geti(session->config, H2_CONF_STREAM_MAX_MEM); + session->max_stream_count = h2_config_geti(session->config, + H2_CONF_MAX_STREAMS); + session->max_stream_mem = h2_config_geti(session->config, + H2_CONF_STREAM_MAX_MEM); status = apr_thread_cond_create(&session->iowait, session->pool); if (status != APR_SUCCESS) { return NULL; } - session->streams = h2_ihash_create(session->pool,offsetof(h2_stream, id)); - session->workers = workers; + session->streams = h2_ihash_create(session->pool, + offsetof(h2_stream, id)); session->mplx = h2_mplx_create(c, session->pool, session->config, session->s->timeout, workers); h2_mplx_set_consumed_cb(session->mplx, update_window, session); /* Install the connection input filter that feeds the session */ - session->cin = h2_filter_cin_create(session->pool, h2_session_receive, session); + session->cin = h2_filter_cin_create(session->pool, + h2_session_receive, session); ap_add_input_filter("H2_IN", session->cin, r, c); h2_conn_io_init(&session->io, c, session->config, session->pool); @@ -844,8 +921,8 @@ static h2_session *h2_session_create_int(conn_rec *c, h2_session_destroy(session); return NULL; } - nghttp2_option_set_peer_max_concurrent_streams(options, - (uint32_t)session->max_stream_count); + nghttp2_option_set_peer_max_concurrent_streams( + options, (uint32_t)session->max_stream_count); /* We need to handle window updates ourself, otherwise we * get flooded by nghttp2. */ nghttp2_option_set_no_auto_window_update(options, 1); @@ -1073,10 +1150,7 @@ static int h2_session_resume_streams_with_data(h2_session *session) h2_stream *h2_session_get_stream(h2_session *session, int stream_id) { - if (!session->last_stream || stream_id != session->last_stream->id) { - session->last_stream = h2_ihash_get(session->streams, stream_id); - } - return session->last_stream; + return h2_ihash_get(session->streams, stream_id); } static ssize_t stream_data_cb(nghttp2_session *ng2s, @@ -1443,9 +1517,6 @@ apr_status_t h2_session_stream_destroy(h2_session *session, h2_stream *stream) h2_mplx_stream_done(session->mplx, stream->id, stream->rst_error); } - if (session->last_stream == stream) { - session->last_stream = NULL; - } if (session->streams) { h2_ihash_remove(session->streams, stream->id); } @@ -1463,10 +1534,11 @@ apr_status_t h2_session_stream_destroy(h2_session *session, h2_stream *stream) int h2_session_push_enabled(h2_session *session) { - /* iff we can and they can */ - return (h2_config_geti(session->config, H2_CONF_PUSH) + /* iff we can and they can and want */ + return (session->remote.accepting /* remote GOAWAY received */ + && h2_config_geti(session->config, H2_CONF_PUSH) && nghttp2_session_get_remote_settings(session->ngh2, - NGHTTP2_SETTINGS_ENABLE_PUSH)); + NGHTTP2_SETTINGS_ENABLE_PUSH)); } static apr_status_t h2_session_send(h2_session *session) @@ -1709,7 +1781,8 @@ static void h2_session_ev_init(h2_session *session, int arg, const char *msg) static void h2_session_ev_local_goaway(h2_session *session, int arg, const char *msg) { - session->local_shutdown = 1; + session->local.accepting = 0; + cleanup_streams(session); switch (session->state) { case H2_SESSION_ST_LOCAL_SHUTDOWN: /* already did that? */ @@ -1727,6 +1800,8 @@ static void h2_session_ev_local_goaway(h2_session *session, int arg, const char static void h2_session_ev_remote_goaway(h2_session *session, int arg, const char *msg) { + session->remote.accepting = 0; + cleanup_streams(session); switch (session->state) { case H2_SESSION_ST_REMOTE_SHUTDOWN: /* already received that? */ @@ -1813,7 +1888,7 @@ static void h2_session_ev_no_io(h2_session *session, int arg, const char *msg) /* When we have no streams, no task event are possible, * switch to blocking reads */ transit(session, "no io", H2_SESSION_ST_IDLE); - session->idle_until = (session->requests_received? + session->idle_until = (session->remote.emitted_count? session->s->keep_alive_timeout : session->s->timeout) + now; session->keep_sync_until = now + apr_time_from_sec(1); @@ -1970,7 +2045,7 @@ static void update_child_status(h2_session *session, int status, const char *msg "%s, streams: %d/%d/%d/%d/%d (open/recv/resp/push/rst)", msg? msg : "-", (int)h2_ihash_count(session->streams), - (int)session->requests_received, + (int)session->remote.emitted_count, (int)session->responses_submitted, (int)session->pushes_submitted, (int)session->pushes_reset + session->streams_reset); @@ -2030,7 +2105,7 @@ apr_status_t h2_session_process(h2_session *session, int async) : SERVER_BUSY_READ), "idle"); /* make certain, the client receives everything before we idle */ if (!session->keep_sync_until - && async && no_streams && !session->r && session->requests_received) { + && async && no_streams && !session->r && session->remote.emitted_count) { ap_log_cerror( APLOG_MARK, APLOG_TRACE1, status, c, "h2_session(%ld): async idle, nonblock read", session->id); /* We do not return to the async mpm immediately, since under @@ -2201,8 +2276,8 @@ apr_status_t h2_session_process(h2_session *session, int async) } else if (status == APR_TIMEUP) { /* go back to checking all inputs again */ - transit(session, "wait cycle", session->local_shutdown? - H2_SESSION_ST_LOCAL_SHUTDOWN : H2_SESSION_ST_BUSY); + transit(session, "wait cycle", session->local.accepting? + H2_SESSION_ST_BUSY : H2_SESSION_ST_LOCAL_SHUTDOWN); } else { ap_log_cerror(APLOG_MARK, APLOG_WARNING, status, c, @@ -2234,6 +2309,10 @@ apr_status_t h2_session_process(h2_session *session, int async) && !nghttp2_session_want_write(session->ngh2)) { dispatch_event(session, H2_SESSION_EV_NGH2_DONE, 0, NULL); } + if (session->reprioritize) { + h2_mplx_reprioritize(session->mplx, stream_pri_cmp, session); + session->reprioritize = 0; + } } out: diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h index 566e79dee2..ea5f82a3b1 100644 --- a/modules/http2/h2_session.h +++ b/modules/http2/h2_session.h @@ -80,18 +80,29 @@ typedef struct h2_session { * of 'h2c', NULL otherwise */ server_rec *s; /* server/vhost we're starting on */ const struct h2_config *config; /* Relevant config for this session */ - + apr_pool_t *pool; /* pool to use in session */ + struct h2_mplx *mplx; /* multiplexer for stream data */ + struct h2_workers *workers; /* for executing stream tasks */ + struct h2_filter_cin *cin; /* connection input filter context */ + h2_conn_io io; /* io on httpd conn filters */ + struct h2_ihash_t *streams; /* streams handled by this session */ + struct nghttp2_session *ngh2; /* the nghttp2 session (internal use) */ + h2_session_state state; /* state session is in */ + + h2_session_props local; /* properties of local session */ + h2_session_props remote; /* properites of remote session */ + unsigned int reprioritize : 1; /* scheduled streams priority changed */ unsigned int eoc_written : 1; /* h2 eoc bucket written */ unsigned int flush : 1; /* flushing output necessary */ - unsigned int local_shutdown: 1; /* GOAWAY has been sent by us */ apr_interval_time_t wait_us; /* timout during BUSY_WAIT state, micro secs */ + struct h2_push_diary *push_diary; /* remember pushes, avoid duplicates */ + int unsent_submits; /* number of submitted, but not yet written responses. */ int unsent_promises; /* number of submitted, but not yet written push promised */ - int requests_received; /* number of http/2 requests received */ int responses_submitted; /* number of http/2 responses submitted */ int streams_reset; /* number of http/2 streams reset by client */ int pushes_promised; /* number of http/2 push promises submitted */ @@ -101,9 +112,6 @@ typedef struct h2_session { apr_size_t frames_received; /* number of http/2 frames received */ apr_size_t frames_sent; /* number of http/2 frames sent */ - int max_stream_received; /* highest stream id created */ - int max_stream_handled; /* highest stream id completed */ - apr_size_t max_stream_count; /* max number of open streams */ apr_size_t max_stream_mem; /* max buffer memory for a single stream */ @@ -111,25 +119,11 @@ typedef struct h2_session { apr_time_t idle_until; /* Time we shut down due to sheer boredom */ apr_time_t keep_sync_until; /* Time we sync wait until passing to async mpm */ - apr_pool_t *pool; /* pool to use in session handling */ apr_bucket_brigade *bbtmp; /* brigade for keeping temporary data */ struct apr_thread_cond_t *iowait; /* our cond when trywaiting for data */ - struct h2_filter_cin *cin; /* connection input filter context */ - h2_conn_io io; /* io on httpd conn filters */ - - struct h2_mplx *mplx; /* multiplexer for stream data */ - - struct h2_stream *last_stream; /* last stream worked with */ - struct h2_ihash_t *streams; /* streams handled by this session */ - apr_pool_t *spare; /* spare stream pool */ - struct nghttp2_session *ngh2; /* the nghttp2 session (internal use) */ - struct h2_workers *workers; /* for executing stream tasks */ - - struct h2_push_diary *push_diary; /* remember pushes, avoid duplicates */ - char status[64]; /* status message for scoreboard */ int last_status_code; /* the one already reported */ const char *last_status_msg; /* the one already reported */ diff --git a/modules/http2/h2_version.h b/modules/http2/h2_version.h index 2cf3d67e9b..836aa07468 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.4.3" +#define MOD_HTTP2_VERSION "1.4.4" /** * @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 0x010403 +#define MOD_HTTP2_VERSION_NUM 0x010404 #endif /* mod_h2_h2_version_h */