On Mon, Feb 6, 2017 at 2:31 PM, Stefan Eissing
<[email protected]> 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);
