From 5b44e011362830de3fee365eca770f6ab0a4ab79 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Mon, 21 Mar 2022 09:54:41 +0000 Subject: *) mod_http2: small improvements from the http1-separation branch that apply in general. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1899107 13f79535-47bb-0310-9956-ffa450edef68 --- modules/http2/h2_bucket_beam.c | 4 +-- modules/http2/h2_bucket_beam.h | 9 ----- modules/http2/h2_c2.c | 82 +++++++++++++++--------------------------- modules/http2/h2_c2.h | 13 ------- modules/http2/h2_mplx.c | 16 +++++---- modules/http2/h2_stream.c | 23 ++++++------ modules/http2/mod_http2.c | 21 ----------- 7 files changed, 51 insertions(+), 117 deletions(-) diff --git a/modules/http2/h2_bucket_beam.c b/modules/http2/h2_bucket_beam.c index 60e48c0f3b..c0968b6d93 100644 --- a/modules/http2/h2_bucket_beam.c +++ b/modules/http2/h2_bucket_beam.c @@ -632,9 +632,9 @@ transfer: brecv = h2_bucket_headers_clone(bsender, bb->p, bb->bucket_alloc); } else if (AP_BUCKET_IS_ERROR(bsender)) { - ap_bucket_error *eb = (ap_bucket_error *)bsender; + ap_bucket_error *eb = bsender->data; brecv = ap_bucket_error_create(eb->status, eb->data, - bb->p, bb->bucket_alloc); + bb->p, bb->bucket_alloc); } } else if (bsender->length == 0) { diff --git a/modules/http2/h2_bucket_beam.h b/modules/http2/h2_bucket_beam.h index 0e5111aab8..b6847733e3 100644 --- a/modules/http2/h2_bucket_beam.h +++ b/modules/http2/h2_bucket_beam.h @@ -27,17 +27,8 @@ struct apr_thread_cond_t; * across threads with as little copying as possible. */ -typedef void h2_beam_mutex_leave(struct apr_thread_mutex_t *lock); - -typedef struct { - apr_thread_mutex_t *mutex; - h2_beam_mutex_leave *leave; -} h2_beam_lock; - typedef struct h2_bucket_beam h2_bucket_beam; -typedef apr_status_t h2_beam_mutex_enter(void *ctx, h2_beam_lock *pbl); - typedef void h2_beam_io_callback(void *ctx, h2_bucket_beam *beam, apr_off_t bytes); typedef void h2_beam_ev_callback(void *ctx, h2_bucket_beam *beam); diff --git a/modules/http2/h2_c2.c b/modules/http2/h2_c2.c index 4a0d189e92..751a509ba2 100644 --- a/modules/http2/h2_c2.c +++ b/modules/http2/h2_c2.c @@ -52,7 +52,6 @@ #include "h2_util.h" -static h2_mpm_type_t mpm_type = H2_MPM_UNKNOWN; static module *mpm_module; static int mpm_supported = 1; static apr_socket_t *dummy_socket; @@ -67,22 +66,18 @@ static void check_modules(int force) module *m = ap_loaded_modules[i]; if (!strcmp("event.c", m->name)) { - mpm_type = H2_MPM_EVENT; mpm_module = m; break; } else if (!strcmp("motorz.c", m->name)) { - mpm_type = H2_MPM_MOTORZ; mpm_module = m; break; } else if (!strcmp("mpm_netware.c", m->name)) { - mpm_type = H2_MPM_NETWARE; mpm_module = m; break; } else if (!strcmp("prefork.c", m->name)) { - mpm_type = H2_MPM_PREFORK; mpm_module = m; /* While http2 can work really well on prefork, it collides * today's use case for prefork: running single-thread app engines @@ -93,18 +88,15 @@ static void check_modules(int force) break; } else if (!strcmp("simple_api.c", m->name)) { - mpm_type = H2_MPM_SIMPLE; mpm_module = m; mpm_supported = 0; break; } else if (!strcmp("mpm_winnt.c", m->name)) { - mpm_type = H2_MPM_WINNT; mpm_module = m; break; } else if (!strcmp("worker.c", m->name)) { - mpm_type = H2_MPM_WORKER; mpm_module = m; break; } @@ -113,12 +105,6 @@ static void check_modules(int force) } } -h2_mpm_type_t h2_conn_mpm_type(void) -{ - check_modules(0); - return mpm_type; -} - const char *h2_conn_mpm_name(void) { check_modules(0); @@ -173,7 +159,7 @@ static apr_status_t h2_c2_filter_in(ap_filter_t* f, h2_conn_ctx_t *conn_ctx; h2_c2_fctx_in_t *fctx = f->ctx; apr_status_t status = APR_SUCCESS; - apr_bucket *b, *next; + apr_bucket *b; apr_off_t bblen; const int trace1 = APLOGctrace1(f->c); apr_size_t rmax = ((readbytes <= APR_SIZE_MAX)? @@ -182,12 +168,6 @@ static apr_status_t h2_c2_filter_in(ap_filter_t* f, conn_ctx = h2_conn_ctx_get(f->c); ap_assert(conn_ctx); - if (trace1) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, f->c, - "h2_c2_in(%s-%d): read, mode=%d, block=%d, readbytes=%ld", - conn_ctx->id, conn_ctx->stream_id, mode, block, (long)readbytes); - } - if (mode == AP_MODE_INIT) { return ap_get_brigade(f->c->input_filters, bb, mode, block, readbytes); } @@ -196,6 +176,12 @@ static apr_status_t h2_c2_filter_in(ap_filter_t* f, return APR_ECONNABORTED; } + if (APLOGctrace1(f->c)) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, f->c, + "h2_c2_in(%s-%d): read, mode=%d, block=%d, readbytes=%ld", + conn_ctx->id, conn_ctx->stream_id, mode, block, (long)readbytes); + } + if (!fctx) { fctx = apr_pcalloc(f->c->pool, sizeof(*fctx)); f->ctx = fctx; @@ -206,20 +192,10 @@ static apr_status_t h2_c2_filter_in(ap_filter_t* f, } } - /* Cleanup brigades from those nasty 0 length non-meta buckets - * that apr_brigade_split_line() sometimes produces. */ - for (b = APR_BRIGADE_FIRST(fctx->bb); - b != APR_BRIGADE_SENTINEL(fctx->bb); b = next) { - next = APR_BUCKET_NEXT(b); - if (b->length == 0 && !APR_BUCKET_IS_METADATA(b)) { - apr_bucket_delete(b); - } - } - while (APR_BRIGADE_EMPTY(fctx->bb)) { /* Get more input data for our request. */ - if (trace1) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, f->c, + if (APLOGctrace1(f->c)) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, f->c, "h2_c2_in(%s-%d): get more data from mplx, block=%d, " "readbytes=%ld", conn_ctx->id, conn_ctx->stream_id, block, (long)readbytes); @@ -245,8 +221,8 @@ receive: status = APR_EOF; } - if (trace1) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, f->c, + if (APLOGctrace3(f->c)) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE3, status, f->c, "h2_c2_in(%s-%d): read returned", conn_ctx->id, conn_ctx->stream_id); } @@ -265,8 +241,8 @@ receive: return status; } - if (trace1) { - h2_util_bb_log(f->c, conn_ctx->stream_id, APLOG_TRACE2, + if (APLOGctrace3(f->c)) { + h2_util_bb_log(f->c, conn_ctx->stream_id, APLOG_TRACE3, "c2 input recv raw", fctx->bb); } if (h2_c_logio_add_bytes_in) { @@ -286,8 +262,8 @@ receive: } if (APR_BRIGADE_EMPTY(fctx->bb)) { - if (trace1) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, f->c, + if (APLOGctrace3(f->c)) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, f->c, "h2_c2_in(%s-%d): no data", conn_ctx->id, conn_ctx->stream_id); } @@ -310,16 +286,14 @@ receive: * though it ends with CRLF and creates a 0 length bucket */ status = apr_brigade_split_line(bb, fctx->bb, block, HUGE_STRING_LEN); - if (APLOGctrace1(f->c)) { + if (APLOGctrace3(f->c)) { char buffer[1024]; apr_size_t len = sizeof(buffer)-1; apr_brigade_flatten(bb, buffer, &len); buffer[len] = 0; - if (trace1) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, f->c, - "h2_c2_in(%s-%d): getline: %s", - conn_ctx->id, conn_ctx->stream_id, buffer); - } + ap_log_cerror(APLOG_MARK, APLOG_TRACE3, status, f->c, + "h2_c2_in(%s-%d): getline: %s", + conn_ctx->id, conn_ctx->stream_id, buffer); } } else { @@ -332,9 +306,9 @@ receive: status = APR_ENOTIMPL; } - if (trace1) { + if (APLOGctrace3(f->c)) { apr_brigade_length(bb, 0, &bblen); - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, f->c, + ap_log_cerror(APLOG_MARK, APLOG_TRACE3, status, f->c, "h2_c2_in(%s-%d): %ld data bytes", conn_ctx->id, conn_ctx->stream_id, (long)bblen); } @@ -615,13 +589,15 @@ static int h2_c2_hook_post_read_request(request_rec *r) static int h2_c2_hook_fixups(request_rec *r) { - /* secondary connection? */ - if (r->connection->master) { - h2_conn_ctx_t *conn_ctx = h2_conn_ctx_get(r->connection); - if (conn_ctx) { - check_push(r, "late_fixup"); - } + conn_rec *c2 = r->connection; + h2_conn_ctx_t *conn_ctx; + + if (!c2->master || !(conn_ctx = h2_conn_ctx_get(c2)) || !conn_ctx->stream_id) { + return DECLINED; } + + check_push(r, "late_fixup"); + return DECLINED; } diff --git a/modules/http2/h2_c2.h b/modules/http2/h2_c2.h index 05454459fb..cfe9755fcc 100644 --- a/modules/http2/h2_c2.h +++ b/modules/http2/h2_c2.h @@ -19,19 +19,6 @@ #include -typedef enum { - H2_MPM_UNKNOWN, - H2_MPM_WORKER, - H2_MPM_EVENT, - H2_MPM_PREFORK, - H2_MPM_MOTORZ, - H2_MPM_SIMPLE, - H2_MPM_NETWARE, - H2_MPM_WINNT, -} h2_mpm_type_t; - -/* Returns the type of MPM module detected */ -h2_mpm_type_t h2_conn_mpm_type(void); const char *h2_conn_mpm_name(void); int h2_mpm_supported(void); diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c index dc930f7eb4..1f1b31d240 100644 --- a/modules/http2/h2_mplx.c +++ b/modules/http2/h2_mplx.c @@ -130,16 +130,18 @@ static void c1c2_stream_joined(h2_mplx *m, h2_stream *stream) static void m_stream_cleanup(h2_mplx *m, h2_stream *stream) { - h2_conn_ctx_t *c2_ctx = stream->c2? h2_conn_ctx_get(stream->c2) : NULL; + h2_conn_ctx_t *c2_ctx = h2_conn_ctx_get(stream->c2); ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c1, H2_STRM_MSG(stream, "cleanup, unsubscribing from beam events")); - if (stream->output) { - h2_beam_on_was_empty(stream->output, NULL, NULL); - } - if (stream->input) { - h2_beam_on_received(stream->input, NULL, NULL); - h2_beam_on_consumed(stream->input, NULL, NULL); + if (c2_ctx) { + if (c2_ctx->beam_out) { + h2_beam_on_was_empty(c2_ctx->beam_out, NULL, NULL); + } + if (c2_ctx->beam_in) { + h2_beam_on_received(c2_ctx->beam_in, NULL, NULL); + h2_beam_on_consumed(c2_ctx->beam_in, NULL, NULL); + } } ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c1, diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c index da8443fe7c..71c005ff2f 100644 --- a/modules/http2/h2_stream.c +++ b/modules/http2/h2_stream.c @@ -197,18 +197,17 @@ static void H2_STREAM_OUT_LOG(int lvl, h2_stream *s, const char *tag) apr_status_t h2_stream_setup_input(h2_stream *stream) { - if (stream->input == NULL) { - int empty = (stream->input_closed - && (!stream->in_buffer - || APR_BRIGADE_EMPTY(stream->in_buffer))); - if (!empty) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, stream->session->c1, - H2_STRM_MSG(stream, "setup input beam")); - h2_beam_create(&stream->input, stream->session->c1, - stream->pool, stream->id, - "input", 0, stream->session->s->timeout); - } - } + /* already done? */ + if (stream->input != NULL) goto cleanup; + /* if already closed and nothing was every sent, leave it */ + if (stream->input_closed && !stream->in_buffer) goto cleanup; + + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, stream->session->c1, + H2_STRM_MSG(stream, "setup input beam")); + h2_beam_create(&stream->input, stream->session->c1, + stream->pool, stream->id, + "input", 0, stream->session->s->timeout); +cleanup: return APR_SUCCESS; } diff --git a/modules/http2/mod_http2.c b/modules/http2/mod_http2.c index 1784b04f16..e7f1002912 100644 --- a/modules/http2/mod_http2.c +++ b/modules/http2/mod_http2.c @@ -125,27 +125,6 @@ static int h2_post_config(apr_pool_t *p, apr_pool_t *plog, myfeats.dyn_windows? "+DWINS" : "", ngh2? ngh2->version_str : "unknown"); - switch (h2_conn_mpm_type()) { - case H2_MPM_SIMPLE: - case H2_MPM_MOTORZ: - case H2_MPM_NETWARE: - case H2_MPM_WINNT: - /* not sure we need something extra for those. */ - break; - case H2_MPM_EVENT: - case H2_MPM_WORKER: - /* all fine, we know these ones */ - break; - case H2_MPM_PREFORK: - /* ok, we now know how to handle that one */ - break; - case H2_MPM_UNKNOWN: - /* ??? */ - ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, s, APLOGNO(03091) - "post_config: mpm type unknown"); - break; - } - if (!h2_mpm_supported() && !mpm_warned) { mpm_warned = 1; ap_log_error(APLOG_MARK, APLOG_WARNING, 0, s, APLOGNO(10034) -- cgit v1.2.3