diff --git a/modules/http2/h2_c1_io.c b/modules/http2/h2_c1_io.c index c239254d3c..7ac1c8d6aa 100644 --- a/modules/http2/h2_c1_io.c +++ b/modules/http2/h2_c1_io.c @@ -456,11 +456,15 @@ static apr_status_t c1_in_feed_brigade(h2_session *session, *inout_len = 0; while (!APR_BRIGADE_EMPTY(bb)) { b = APR_BRIGADE_FIRST(bb); + APR_BUCKET_REMOVE(b); if (!APR_BUCKET_IS_METADATA(b)) { rv = c1_in_feed_bucket(session, b, inout_len); - if (APR_SUCCESS != rv) goto cleanup; + if (APR_SUCCESS != rv) { + apr_bucket_destroy(b); + goto cleanup; + } } - apr_bucket_delete(b); + apr_bucket_destroy(b); } cleanup: apr_brigade_cleanup(bb); diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c index c50e15ccd5..c008122db6 100644 --- a/modules/http2/h2_mplx.c +++ b/modules/http2/h2_mplx.c @@ -148,7 +148,13 @@ 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 = h2_conn_ctx_get(stream->c2); + h2_conn_ctx_t *c2_ctx; + + if (stream->mplx_cleanup_done) return; + stream->mplx_cleanup_done = 1; + m->in_stream_cleanup = 1; + + 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")); @@ -195,6 +201,7 @@ static void m_stream_cleanup(h2_mplx *m, h2_stream *stream) ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, m->c1, H2_STRM_MSG(stream, "cleanup, never started, move to spurge")); } + m->in_stream_cleanup = 0; } static h2_c2_transit *c2_transit_create(h2_mplx *m) @@ -278,6 +285,7 @@ h2_mplx *h2_mplx_c1_create(int child_num, apr_uint32_t id, h2_stream *stream0, apr_allocator_t *allocator; apr_thread_mutex_t *mutex = NULL; h2_mplx *m = NULL; + apr_pool_t *poll_pool; m = apr_pcalloc(parent, sizeof(h2_mplx)); m->stream0 = stream0; @@ -337,8 +345,14 @@ h2_mplx *h2_mplx_c1_create(int child_num, apr_uint32_t id, h2_stream *stream0, m->streams_ev_in = apr_array_make(m->pool, 10, sizeof(h2_stream*)); m->streams_ev_out = apr_array_make(m->pool, 10, sizeof(h2_stream*)); - m->streams_input_read = h2_iq_create(m->pool, 10); - m->streams_output_written = h2_iq_create(m->pool, 10); + /* iqueues MUST use a separate pool: iq_grow allocates via apr_pcalloc + * under poll_lock, while other m->pool allocations (spurge growth etc.) + * happen under m->lock. apr_palloc is NOT thread-safe on the same pool + * under different locks -- the bump pointer can hand out overlapping + * memory to concurrent callers. */ + apr_pool_create(&poll_pool, m->pool); + m->streams_input_read = h2_iq_create(poll_pool, m->max_streams); + m->streams_output_written = h2_iq_create(poll_pool, m->max_streams); status = apr_thread_mutex_create(&m->poll_lock, APR_THREAD_MUTEX_DEFAULT, m->pool); if (APR_SUCCESS != status) goto failure; @@ -500,6 +514,7 @@ static int m_stream_cancel_iter(void *ctx, void *val) { /* Reset, should transit to CLOSED state */ h2_stream_rst(stream, H2_ERR_NO_ERROR); /* All connection data has been sent, simulate cleanup */ + m->in_stream_cleanup = 1; h2_stream_dispatch(stream, H2_SEV_EOS_SENT); m_stream_cleanup(m, stream); return 0; @@ -761,7 +776,7 @@ void h2_mplx_c1_process(h2_mplx *m, while ((sid = h2_iq_shift(ready_to_process)) > 0) { h2_stream *stream = get_stream(session, sid); - if (stream) { + if (stream && stream->state < H2_SS_CLEANUP) { ap_assert(!stream->scheduled); rv = c1_process_stream(session->mplx, stream, stream_pri_cmp, session); if (APR_SUCCESS != rv) { @@ -1185,6 +1200,7 @@ apr_status_t h2_mplx_c1_client_rst(h2_mplx *m, int stream_id, h2_stream *stream) H2_STRM_MSG(stream, "very early RST, drop")); h2_stream_set_monitor(stream, NULL); h2_stream_rst(stream, H2_ERR_STREAM_CLOSED); + m->in_stream_cleanup = 1; h2_stream_dispatch(stream, H2_SEV_EOS_SENT); m_stream_cleanup(m, stream); m_be_annoyed(m); diff --git a/modules/http2/h2_mplx.h b/modules/http2/h2_mplx.h index 12e36f766f..cdae179d96 100644 --- a/modules/http2/h2_mplx.h +++ b/modules/http2/h2_mplx.h @@ -60,6 +60,7 @@ struct h2_mplx { apr_uint32_t id; /* id unique per child */ conn_rec *c1; /* the main connection */ apr_pool_t *pool; + int in_stream_cleanup; struct h2_stream *stream0; /* HTTP/2's stream 0 */ server_rec *s; /* server for master conn */ diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c index a2ddab8d57..5f32454363 100644 --- a/modules/http2/h2_session.c +++ b/modules/http2/h2_session.c @@ -99,7 +99,7 @@ static int rst_unprocessed_stream(h2_stream *stream, void *ctx) (!stream->session->remote.accepting && stream->id > stream->session->remote.accepted_max)) ); - if (unprocessed) { + if (unprocessed && stream->state < H2_SS_CLEANUP) { h2_stream_rst(stream, H2_ERR_NO_ERROR); return 0; } @@ -269,7 +269,7 @@ static int on_stream_close_cb(nghttp2_session *ngh2, int32_t stream_id, (void)ngh2; stream = get_stream(session, stream_id); - if (stream) { + if (stream && stream->state < H2_SS_CLEANUP) { if (error_code) { ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c1, H2_STRM_LOG(APLOGNO(03065), stream, @@ -512,6 +512,7 @@ static int on_send_data_cb(nghttp2_session *ngh2, h2_stream *stream; apr_bucket *b; apr_off_t len = length; + apr_bucket_brigade *bb; (void)ngh2; (void)source; @@ -542,29 +543,31 @@ static int on_send_data_cb(nghttp2_session *ngh2, return NGHTTP2_ERR_CALLBACK_FAILURE; } - status = h2_stream_read_to(stream, session->bbtmp, &len, &eos); + bb = apr_brigade_create(session->pool, + session->c1->bucket_alloc); + status = h2_stream_read_to(stream, bb, &len, &eos); if (status != APR_SUCCESS) { ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, session->c1, H2_STRM_MSG(stream, "send_data_cb, reading stream")); - apr_brigade_cleanup(session->bbtmp); + apr_brigade_destroy(bb); return NGHTTP2_ERR_CALLBACK_FAILURE; } else if (len != (apr_off_t)length) { ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, session->c1, H2_STRM_MSG(stream, "send_data_cb, wanted %ld bytes, " "got %ld from stream"), (long)length, (long)len); - apr_brigade_cleanup(session->bbtmp); + apr_brigade_destroy(bb); return NGHTTP2_ERR_CALLBACK_FAILURE; } - + if (padlen) { - b = apr_bucket_immortal_create(immortal_zeros, padlen, + b = apr_bucket_immortal_create(immortal_zeros, padlen, session->c1->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(session->bbtmp, b); + APR_BRIGADE_INSERT_TAIL(bb, b); } - - status = h2_c1_io_append(&session->io, session->bbtmp); - apr_brigade_cleanup(session->bbtmp); + + status = h2_c1_io_append(&session->io, bb); + apr_brigade_destroy(bb); if (status == APR_SUCCESS) { stream->out_data_frames++; @@ -1708,6 +1711,7 @@ static void ev_stream_open(h2_session *session, h2_stream *stream) static void ev_stream_closed(h2_session *session, h2_stream *stream) { apr_bucket *b; + apr_bucket_brigade *bb; if (H2_STREAM_CLIENT_INITIATED(stream->id) && (stream->id > session->local.completed_max)) { @@ -1721,9 +1725,11 @@ static void ev_stream_closed(h2_session *session, h2_stream *stream) ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, session->c1, H2_STRM_MSG(stream, "adding h2_eos to c1 out")); b = h2_bucket_eos_create(session->c1->bucket_alloc, stream); - APR_BRIGADE_INSERT_TAIL(session->bbtmp, b); - h2_c1_io_append(&session->io, session->bbtmp); - apr_brigade_cleanup(session->bbtmp); + bb = apr_brigade_create(session->pool, + session->c1->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(bb, b); + h2_c1_io_append(&session->io, bb); + apr_brigade_destroy(bb); } static void on_stream_state_enter(void *ctx, h2_stream *stream) @@ -1761,10 +1767,12 @@ static void on_stream_state_enter(void *ctx, h2_stream *stream) break; case H2_SS_CLEANUP: nghttp2_session_set_stream_user_data(session->ngh2, stream->id, NULL); - update_child_status(session, SERVER_BUSY_WRITE, "done", stream); - h2_mplx_c1_stream_cleanup(session->mplx, stream, &session->open_streams); - stream = NULL; - ++session->streams_done; + if (!session->mplx->in_stream_cleanup) { + update_child_status(session, SERVER_BUSY_WRITE, "done", stream); + h2_mplx_c1_stream_cleanup(session->mplx, stream, &session->open_streams); + stream = NULL; + ++session->streams_done; + } break; default: break; diff --git a/modules/http2/h2_stream.h b/modules/http2/h2_stream.h index 405978a44d..adf64d39e9 100644 --- a/modules/http2/h2_stream.h +++ b/modules/http2/h2_stream.h @@ -111,6 +111,7 @@ struct h2_stream { unsigned int aborted : 1; /* was aborted */ unsigned int scheduled : 1; /* stream has been scheduled */ unsigned int input_closed : 1; /* no more request data/trailers coming */ + unsigned int mplx_cleanup_done : 1; unsigned int push_policy; /* which push policy to use for this request */ unsigned int sent_trailers : 1; /* trailers have been submitted */ unsigned int output_eos : 1; /* output EOS in buffer/sent */