On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing <stefan.eiss...@greenbytes.de> wrote: > Currently running some tests. Have crashes on the original patch in my test > suite. Fixed one, hunting for the next...
I think it comes from my change that creates slave connections from master->pool (instead of mplx's), because now slave's pool is already destroyed when h2_mplx_release_and_join()->task_destroy()->h2_slave_destroy() is called (hence the crash). I restored your original code in this new (attached) patch. @s.priebe, would you test this one please?
Index: server/mpm/event/event.c =================================================================== --- server/mpm/event/event.c (revision 1781789) +++ server/mpm/event/event.c (working copy) @@ -1743,6 +1743,8 @@ static void * APR_THREAD_FUNC listener_thread(apr_ enable_listensocks(process_slot); } if (!listeners_disabled) { + apr_thread_mutex_t *mutex; + lr = (ap_listen_rec *) pt->baton; ap_pop_pool(&ptrans, worker_queue_info); @@ -1751,19 +1753,24 @@ static void * APR_THREAD_FUNC listener_thread(apr_ apr_allocator_t *allocator; apr_allocator_create(&allocator); - apr_allocator_max_free_set(allocator, - ap_max_mem_free); - apr_pool_create_ex(&ptrans, pconf, NULL, allocator); - apr_allocator_owner_set(allocator, ptrans); - if (ptrans == NULL) { + apr_allocator_max_free_set(allocator, ap_max_mem_free); + rc = apr_pool_create_ex(&ptrans, pconf, NULL, + allocator); + if (rc != APR_SUCCESS) { ap_log_error(APLOG_MARK, APLOG_CRIT, rc, ap_server_conf, APLOGNO(03097) "Failed to create transaction pool"); + apr_allocator_destroy(allocator); signal_threads(ST_GRACEFUL); return NULL; } + apr_allocator_owner_set(allocator, ptrans); + apr_pool_tag(ptrans, "transaction"); } - apr_pool_tag(ptrans, "transaction"); + apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, + ptrans); + apr_allocator_mutex_set(apr_pool_allocator_get(ptrans), + mutex); get_worker(&have_idle_worker, 1, &workers_were_busy); rc = lr->accept_func(&csd, lr, ptrans); Index: modules/http2/h2_conn.c =================================================================== --- modules/http2/h2_conn.c (revision 1781789) +++ modules/http2/h2_conn.c (working copy) @@ -26,6 +26,8 @@ #include <http_protocol.h> #include <http_request.h> +#include "mpm_common.h" /* for ap_max_mem_free */ + #include "h2_private.h" #include "h2.h" #include "h2_config.h" @@ -250,6 +252,7 @@ apr_status_t h2_conn_pre_close(struct h2_ctx *ctx, conn_rec *h2_slave_create(conn_rec *master, int slave_id, apr_pool_t *parent) { apr_allocator_t *allocator; + apr_thread_mutex_t *mutex; apr_pool_t *pool; conn_rec *c; void *cfg; @@ -264,14 +267,18 @@ conn_rec *h2_slave_create(conn_rec *master, int sl * another thread. */ apr_allocator_create(&allocator); + apr_allocator_max_free_set(allocator, ap_max_mem_free); apr_pool_create_ex(&pool, parent, NULL, allocator); + apr_allocator_owner_set(allocator, pool); apr_pool_tag(pool, "h2_slave_conn"); - apr_allocator_owner_set(allocator, pool); + apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT, pool); + apr_allocator_mutex_set(allocator, mutex); 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_task: creating conn"); + apr_pool_destroy(pool); return NULL; } Index: modules/http2/h2_mplx.c =================================================================== --- modules/http2/h2_mplx.c (revision 1781789) +++ modules/http2/h2_mplx.c (working copy) @@ -33,6 +33,7 @@ #include "h2_private.h" #include "h2_bucket_beam.h" #include "h2_config.h" +#include "h2_session.h" #include "h2_conn.h" #include "h2_ctx.h" #include "h2_h2.h" @@ -259,32 +260,27 @@ static void h2_mplx_destroy(h2_mplx *m) * their HTTP/1 cousins, the separate allocator seems to work better * than protecting a shared h2_session one with an own lock. */ -h2_mplx *h2_mplx_create(conn_rec *c, apr_pool_t *parent, - const h2_config *conf, - apr_interval_time_t stream_timeout, - h2_workers *workers) +h2_mplx *h2_mplx_create(h2_session *session, h2_workers *workers) { apr_status_t status = APR_SUCCESS; - apr_allocator_t *allocator = NULL; + apr_pool_t *parent = session->pool; + const h2_config *conf = session->config; h2_mplx *m; ap_assert(conf); - status = apr_allocator_create(&allocator); - if (status != APR_SUCCESS) { - return NULL; - } - m = apr_pcalloc(parent, sizeof(h2_mplx)); if (m) { + conn_rec *c = session->c; + m->id = c->id; APR_RING_ELEM_INIT(m, link); m->c = c; - apr_pool_create_ex(&m->pool, parent, NULL, allocator); + + apr_pool_create(&m->pool, parent); if (!m->pool) { return NULL; } apr_pool_tag(m->pool, "h2_mplx"); - apr_allocator_owner_set(allocator, m->pool); status = apr_thread_mutex_create(&m->lock, APR_THREAD_MUTEX_DEFAULT, m->pool); @@ -311,7 +307,7 @@ static void h2_mplx_destroy(h2_mplx *m) m->tasks = h2_ihash_create(m->pool, offsetof(h2_task,stream_id)); m->redo_tasks = h2_ihash_create(m->pool, offsetof(h2_task, stream_id)); - m->stream_timeout = stream_timeout; + m->stream_timeout = session->s->timeout; m->workers = workers; m->workers_max = workers->max_workers; m->workers_limit = 6; /* the original h1 max parallel connections */ Index: modules/http2/h2_mplx.h =================================================================== --- modules/http2/h2_mplx.h (revision 1781789) +++ modules/http2/h2_mplx.h (working copy) @@ -38,7 +38,7 @@ struct apr_pool_t; struct apr_thread_mutex_t; struct apr_thread_cond_t; struct h2_bucket_beam; -struct h2_config; +struct h2_session; struct h2_ihash_t; struct h2_task; struct h2_stream; @@ -124,9 +124,7 @@ apr_status_t h2_mplx_child_init(apr_pool_t *pool, * Create the multiplexer for the given HTTP2 session. * Implicitly has reference count 1. */ -h2_mplx *h2_mplx_create(conn_rec *c, apr_pool_t *master, - const struct h2_config *conf, - apr_interval_time_t stream_timeout, +h2_mplx *h2_mplx_create(struct h2_session *session, struct h2_workers *workers); /** Index: modules/http2/h2_session.c =================================================================== --- modules/http2/h2_session.c (revision 1781789) +++ modules/http2/h2_session.c (working copy) @@ -920,8 +920,7 @@ static h2_session *h2_session_create_int(conn_rec return NULL; } - session->mplx = h2_mplx_create(c, session->pool, session->config, - session->s->timeout, workers); + session->mplx = h2_mplx_create(session, workers); h2_mplx_set_consumed_cb(session->mplx, update_window, session);