From 825de6a46027b2f4c30d7ff5a0c8b852d639c207 Mon Sep 17 00:00:00 2001 From: Stefan Eissing Date: Fri, 25 Jan 2019 12:13:47 +0100 Subject: [PATCH] * Fixed keepalives counter on slave connections. --- ChangeLog | 4 + configure.ac | 2 +- mod_http2/h2_conn.c | 11 +- mod_http2/h2_conn.c.orig | 366 --------------------------------------- mod_http2/h2_mplx.c | 5 + mod_http2/h2_task.c | 2 +- mod_http2/h2_version.h | 4 +- 7 files changed, 19 insertions(+), 375 deletions(-) delete mode 100644 mod_http2/h2_conn.c.orig diff --git a/ChangeLog b/ChangeLog index 487d3713..a4e231dd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +v1.12.2 +-------------------------------------------------------------------------------- + * Fixed keepalives counter on slave connections. + v1.12.1 -------------------------------------------------------------------------------- * Fixed Bug introduce in v1.12.0 where mod_proxy_http2 no longer set the server name diff --git a/configure.ac b/configure.ac index ee9b2085..b1e88989 100644 --- a/configure.ac +++ b/configure.ac @@ -14,7 +14,7 @@ # AC_PREREQ([2.69]) -AC_INIT([mod_http2], [1.12.1], [stefan.eissing@greenbytes.de]) +AC_INIT([mod_http2], [1.12.2], [stefan.eissing@greenbytes.de]) LT_PREREQ([2.2.6]) LT_INIT() diff --git a/mod_http2/h2_conn.c b/mod_http2/h2_conn.c index a9f34a75..d29cd7e9 100644 --- a/mod_http2/h2_conn.c +++ b/mod_http2/h2_conn.c @@ -311,6 +311,7 @@ conn_rec *h2_slave_create(conn_rec *master, int slave_id, apr_pool_t *parent) c->notes = apr_table_make(pool, 5); c->input_filters = NULL; c->output_filters = NULL; + c->keepalives = 0; #if AP_MODULE_MAGIC_AT_LEAST(20180903, 1) c->filter_conn_ctx = NULL; #endif @@ -343,16 +344,15 @@ conn_rec *h2_slave_create(conn_rec *master, int slave_id, apr_pool_t *parent) ap_set_module_config(c->conn_config, mpm, cfg); } - ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, - "h2_stream(%ld-%d): created slave", master->id, slave_id); + ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, c, + "h2_slave(%s): created", c->log_id); return c; } void h2_slave_destroy(conn_rec *slave) { - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, slave, - "h2_stream(%s): destroy slave", - apr_table_get(slave->notes, H2_TASK_ID_NOTE)); + ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, slave, + "h2_slave(%s): destroy", slave->log_id); slave->sbh = NULL; apr_pool_destroy(slave->pool); } @@ -376,6 +376,7 @@ apr_status_t h2_slave_run_pre_connection(conn_rec *slave, apr_socket_t *csd) slave->keepalive = AP_CONN_CLOSE; return ap_run_pre_connection(slave, csd); } + ap_assert(slave->output_filters); return APR_SUCCESS; } diff --git a/mod_http2/h2_conn.c.orig b/mod_http2/h2_conn.c.orig deleted file mode 100644 index a544e682..00000000 --- a/mod_http2/h2_conn.c.orig +++ /dev/null @@ -1,366 +0,0 @@ -/* Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -#include -#include - -#include -#include - -#include -#include -#include -#include -#include -#include -#include - -#include - -#include "h2_private.h" -#include "h2.h" -#include "h2_config.h" -#include "h2_ctx.h" -#include "h2_filter.h" -#include "h2_mplx.h" -#include "h2_session.h" -#include "h2_stream.h" -#include "h2_h2.h" -#include "h2_task.h" -#include "h2_workers.h" -#include "h2_conn.h" -#include "h2_version.h" - -static struct h2_workers *workers; - -static h2_mpm_type_t mpm_type = H2_MPM_UNKNOWN; -static module *mpm_module; -static int async_mpm; -static int mpm_supported = 1; -static apr_socket_t *dummy_socket; - -static void check_modules(int force) -{ - static int checked = 0; - int i; - - if (force || !checked) { - for (i = 0; ap_loaded_modules[i]; ++i) { - 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: runnning single-thread app engines - * like php. If we restrict h2_workers to 1 per process, php will - * work fine, but browser will be limited to 1 active request at a - * time. */ - mpm_supported = 0; - 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; - } - } - checked = 1; - } -} - -apr_status_t h2_conn_child_init(apr_pool_t *pool, server_rec *s) -{ - const h2_config *config = h2_config_sget(s); - apr_status_t status = APR_SUCCESS; - int minw, maxw; - int max_threads_per_child = 0; - int idle_secs = 0; - - check_modules(1); - ap_mpm_query(AP_MPMQ_MAX_THREADS, &max_threads_per_child); - - status = ap_mpm_query(AP_MPMQ_IS_ASYNC, &async_mpm); - if (status != APR_SUCCESS) { - /* some MPMs do not implemnent this */ - async_mpm = 0; - status = APR_SUCCESS; - } - - h2_config_init(pool); - - h2_get_num_workers(s, &minw, &maxw); - - idle_secs = h2_config_geti(config, H2_CONF_MAX_WORKER_IDLE_SECS); - ap_log_error(APLOG_MARK, APLOG_TRACE3, 0, s, - "h2_workers: min=%d max=%d, mthrpchild=%d, idle_secs=%d", - minw, maxw, max_threads_per_child, idle_secs); - workers = h2_workers_create(s, pool, minw, maxw, idle_secs); - - ap_register_input_filter("H2_IN", h2_filter_core_input, - NULL, AP_FTYPE_CONNECTION); - - status = h2_mplx_child_init(pool, s); - - if (status == APR_SUCCESS) { - status = apr_socket_create(&dummy_socket, APR_INET, SOCK_STREAM, - APR_PROTO_TCP, pool); - } - - return status; -} - -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); - return mpm_module? mpm_module->name : "unknown"; -} - -int h2_mpm_supported(void) -{ - check_modules(0); - return mpm_supported; -} - -static module *h2_conn_mpm_module(void) -{ - check_modules(0); - return mpm_module; -} - -apr_status_t h2_conn_setup(h2_ctx *ctx, conn_rec *c, request_rec *r) -{ - h2_session *session; - apr_status_t status; - - if (!workers) { - ap_log_cerror(APLOG_MARK, APLOG_ERR, 0, c, APLOGNO(02911) - "workers not initialized"); - return APR_EGENERAL; - } - - if (r) { - status = h2_session_rcreate(&session, r, ctx, workers); - } - else { - status = h2_session_create(&session, c, ctx, workers); - } - - if (status == APR_SUCCESS) { - h2_ctx_session_set(ctx, session); - } - return status; -} - -apr_status_t h2_conn_run(struct h2_ctx *ctx, conn_rec *c) -{ - apr_status_t status; - int mpm_state = 0; - h2_session *session = h2_ctx_session_get(ctx); - - ap_assert(session); - do { - if (c->cs) { - c->cs->sense = CONN_SENSE_DEFAULT; - c->cs->state = CONN_STATE_HANDLER; - } - - status = h2_session_process(session, async_mpm); - - if (APR_STATUS_IS_EOF(status)) { - ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c, - H2_SSSN_LOG(APLOGNO(03045), session, - "process, closing conn")); - c->keepalive = AP_CONN_CLOSE; - } - else { - c->keepalive = AP_CONN_KEEPALIVE; - } - - if (ap_mpm_query(AP_MPMQ_MPM_STATE, &mpm_state)) { - break; - } - } while (!async_mpm - && c->keepalive == AP_CONN_KEEPALIVE - && mpm_state != AP_MPMQ_STOPPING); - - if (c->cs) { - switch (session->state) { - case H2_SESSION_ST_INIT: - case H2_SESSION_ST_IDLE: - case H2_SESSION_ST_BUSY: - case H2_SESSION_ST_WAIT: - c->cs->state = CONN_STATE_WRITE_COMPLETION; - break; - case H2_SESSION_ST_CLEANUP: - case H2_SESSION_ST_DONE: - default: - c->cs->state = CONN_STATE_LINGER; - break; - } - } - - return APR_SUCCESS; -} - -apr_status_t h2_conn_pre_close(struct h2_ctx *ctx, conn_rec *c) -{ - h2_session *session = h2_ctx_session_get(ctx); - - (void)c; - if (session) { - apr_status_t status = h2_session_pre_close(session, async_mpm); - return (status == APR_SUCCESS)? DONE : status; - } - return DONE; -} - -conn_rec *h2_slave_create(conn_rec *master, int slave_id, apr_pool_t *parent) -{ - apr_allocator_t *allocator; - apr_status_t status; - apr_pool_t *pool; - conn_rec *c; - void *cfg; - module *mpm; - - ap_assert(master); - ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, master, - "h2_stream(%ld-%d): create slave", master->id, slave_id); - - /* We create a pool with its own allocator to be used for - * processing a request. This is the only way to have the processing - * independant of its parent pool in the sense that it can work in - * another thread. Also, the new allocator needs its own mutex to - * synchronize sub-pools. - */ - apr_allocator_create(&allocator); - apr_allocator_max_free_set(allocator, ap_max_mem_free); - status = apr_pool_create_ex(&pool, parent, NULL, allocator); - if (status != APR_SUCCESS) { - ap_log_cerror(APLOG_MARK, APLOG_ERR, status, master, - APLOGNO(10004) "h2_session(%ld-%d): create slave pool", - master->id, slave_id); - return NULL; - } - apr_allocator_owner_set(allocator, pool); - apr_pool_tag(pool, "h2_slave_conn"); - - c = (conn_rec *) apr_palloc(pool, sizeof(conn_rec)); - if (c == NULL) { - ap_log_cerror(APLOG_MARK, APLOG_ERR, APR_ENOMEM, master, - APLOGNO(02913) "h2_session(%ld-%d): create slave", - master->id, slave_id); - apr_pool_destroy(pool); - return NULL; - } - - memcpy(c, master, sizeof(conn_rec)); - - c->master = master; - c->pool = pool; - c->conn_config = ap_create_conn_config(pool); - c->notes = apr_table_make(pool, 5); - c->input_filters = NULL; - c->output_filters = NULL; - c->bucket_alloc = apr_bucket_alloc_create(pool); -#if !AP_MODULE_MAGIC_AT_LEAST(20180720, 1) - c->data_in_input_filters = 0; - c->data_in_output_filters = 0; -#endif - /* prevent mpm_event from making wrong assumptions about this connection, - * like e.g. using its socket for an async read check. */ - c->clogging_input_filters = 1; - c->log = NULL; - c->log_id = apr_psprintf(pool, "%ld-%d", - master->id, slave_id); - c->aborted = 0; - /* We cannot install the master connection socket on the slaves, as - * modules mess with timeouts/blocking of the socket, with - * unwanted side effects to the master connection processing. - * Fortunately, since we never use the slave socket, we can just install - * a single, process-wide dummy and everyone is happy. - */ - ap_set_module_config(c->conn_config, &core_module, dummy_socket); - /* TODO: these should be unique to this thread */ - c->sbh = master->sbh; - /* TODO: not all mpm modules have learned about slave connections yet. - * copy their config from master to slave. - */ - if ((mpm = h2_conn_mpm_module()) != NULL) { - cfg = ap_get_module_config(master->conn_config, mpm); - ap_set_module_config(c->conn_config, mpm, cfg); - } - - ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, - "h2_stream(%ld-%d): created slave", master->id, slave_id); - return c; -} - -void h2_slave_destroy(conn_rec *slave) -{ - ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, slave, - "h2_stream(%s): destroy slave", - apr_table_get(slave->notes, H2_TASK_ID_NOTE)); - slave->sbh = NULL; - apr_pool_destroy(slave->pool); -} - -apr_status_t h2_slave_run_pre_connection(conn_rec *slave, apr_socket_t *csd) -{ - if (slave->keepalives == 0) { - /* Simulate that we had already a request on this connection. Some - * hooks trigger special behaviour when keepalives is 0. - * (Not necessarily in pre_connection, but later. Set it here, so it - * is in place.) */ - slave->keepalives = 1; - return ap_run_pre_connection(slave, csd); - } - return APR_SUCCESS; -} - diff --git a/mod_http2/h2_mplx.c b/mod_http2/h2_mplx.c index 77f8dd6a..b8284429 100644 --- a/mod_http2/h2_mplx.c +++ b/mod_http2/h2_mplx.c @@ -431,6 +431,8 @@ void h2_mplx_release_and_join(h2_mplx *m, apr_thread_cond_t *wait) apr_status_t status; int i, wait_secs = 60; + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c, + "h2_mplx(%ld): start release", m->id); /* How to shut down a h2 connection: * 0. abort and tell the workers that no more tasks will come from us */ m->aborted = 1; @@ -980,6 +982,9 @@ static apr_status_t unschedule_slow_tasks(h2_mplx *m) */ n = (m->tasks_active - m->limit_active - (int)h2_ihash_count(m->sredo)); while (n > 0 && (stream = get_latest_repeatable_unsubmitted_stream(m))) { + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c, + "h2_mplx(%s): unschedule, resetting task for redo later", + stream->task->id); h2_task_rst(stream->task, H2_ERR_CANCEL); h2_ihash_add(m->sredo, stream); --n; diff --git a/mod_http2/h2_task.c b/mod_http2/h2_task.c index 430f0898..b67f7a1c 100644 --- a/mod_http2/h2_task.c +++ b/mod_http2/h2_task.c @@ -504,7 +504,7 @@ static int h2_task_pre_conn(conn_rec* c, void *arg) (void)arg; if (ctx->task) { ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, - "h2_h2, pre_connection, found stream task"); + "h2_slave(%s), pre_connection, adding filters", c->log_id); ap_add_input_filter("H2_SLAVE_IN", NULL, NULL, c); ap_add_output_filter("H2_PARSE_H1", NULL, NULL, c); ap_add_output_filter("H2_SLAVE_OUT", NULL, NULL, c); diff --git a/mod_http2/h2_version.h b/mod_http2/h2_version.h index f42259a6..c0daf0b8 100644 --- a/mod_http2/h2_version.h +++ b/mod_http2/h2_version.h @@ -27,7 +27,7 @@ * @macro * Version number of the http2 module as c string */ -#define MOD_HTTP2_VERSION "1.12.1-git" +#define MOD_HTTP2_VERSION "1.12.2-git" /** * @macro @@ -35,7 +35,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 0x010c01 +#define MOD_HTTP2_VERSION_NUM 0x010c02 #endif /* mod_h2_h2_version_h */