diff options
author | Stefan Eissing <icing@apache.org> | 2016-11-03 00:50:42 +0100 |
---|---|---|
committer | Stefan Eissing <icing@apache.org> | 2016-11-03 00:50:42 +0100 |
commit | 29acf46076c623c0ee2ccc00a24322f791697381 (patch) | |
tree | c1ffe4c1393c578c1d148f2b5805961095842197 | |
parent | generated the HTML file (diff) | |
download | apache2-29acf46076c623c0ee2ccc00a24322f791697381.tar.xz apache2-29acf46076c623c0ee2ccc00a24322f791697381.zip |
mod_http2: fix for beam double cleanup crashes introduced in 1.7.7
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1767803 13f79535-47bb-0310-9956-ffa450edef68
-rw-r--r-- | modules/http2/h2_bucket_beam.c | 268 | ||||
-rw-r--r-- | modules/http2/h2_bucket_beam.h | 63 | ||||
-rw-r--r-- | modules/http2/h2_mplx.c | 10 | ||||
-rw-r--r-- | modules/http2/h2_stream.c | 4 |
4 files changed, 190 insertions, 155 deletions
diff --git a/modules/http2/h2_bucket_beam.c b/modules/http2/h2_bucket_beam.c index 22b2e909c9..9b9c3b3a2f 100644 --- a/modules/http2/h2_bucket_beam.c +++ b/modules/http2/h2_bucket_beam.c @@ -260,8 +260,8 @@ static apr_size_t calc_buffered(h2_bucket_beam *beam) { apr_size_t len = 0; apr_bucket *b; - for (b = H2_BLIST_FIRST(&beam->red); - b != H2_BLIST_SENTINEL(&beam->red); + for (b = H2_BLIST_FIRST(&beam->send_list); + b != H2_BLIST_SENTINEL(&beam->send_list); b = APR_BUCKET_NEXT(b)) { if (b->length == ((apr_size_t)-1)) { /* do not count */ @@ -276,14 +276,14 @@ static apr_size_t calc_buffered(h2_bucket_beam *beam) return len; } -static void r_purge_reds(h2_bucket_beam *beam) +static void r_purge_sent(h2_bucket_beam *beam) { - apr_bucket *bred; - /* delete all red buckets in purge brigade, needs to be called - * from red thread only */ - while (!H2_BLIST_EMPTY(&beam->purge)) { - bred = H2_BLIST_FIRST(&beam->purge); - apr_bucket_delete(bred); + apr_bucket *b; + /* delete all sender buckets in purge brigade, needs to be called + * from sender thread only */ + while (!H2_BLIST_EMPTY(&beam->purge_list)) { + b = H2_BLIST_FIRST(&beam->purge_list); + apr_bucket_delete(b); } } @@ -318,7 +318,7 @@ static apr_status_t r_wait_space(h2_bucket_beam *beam, apr_read_type_e block, if (APR_STATUS_IS_TIMEUP(status)) { return status; } - r_purge_reds(beam); + r_purge_sent(beam); *premain = calc_space_left(beam); } return beam->aborted? APR_ECONNABORTED : APR_SUCCESS; @@ -333,34 +333,34 @@ static void h2_beam_emitted(h2_bucket_beam *beam, h2_beam_proxy *proxy) /* even when beam buckets are split, only the one where * refcount drops to 0 will call us */ H2_BPROXY_REMOVE(proxy); - /* invoked from green thread, the last beam bucket for the red - * bucket bred is about to be destroyed. + /* invoked from receiver thread, the last beam bucket for the send + * bucket is about to be destroyed. * remove it from the hold, where it should be now */ if (proxy->bred) { - for (b = H2_BLIST_FIRST(&beam->hold); - b != H2_BLIST_SENTINEL(&beam->hold); + for (b = H2_BLIST_FIRST(&beam->hold_list); + b != H2_BLIST_SENTINEL(&beam->hold_list); b = APR_BUCKET_NEXT(b)) { if (b == proxy->bred) { break; } } - if (b != H2_BLIST_SENTINEL(&beam->hold)) { + if (b != H2_BLIST_SENTINEL(&beam->hold_list)) { /* bucket is in hold as it should be, mark this one * and all before it for purging. We might have placed meta * buckets without a green proxy into the hold before it * and schedule them for purging now */ - for (b = H2_BLIST_FIRST(&beam->hold); - b != H2_BLIST_SENTINEL(&beam->hold); + for (b = H2_BLIST_FIRST(&beam->hold_list); + b != H2_BLIST_SENTINEL(&beam->hold_list); b = next) { next = APR_BUCKET_NEXT(b); if (b == proxy->bred) { APR_BUCKET_REMOVE(b); - H2_BLIST_INSERT_TAIL(&beam->purge, b); + H2_BLIST_INSERT_TAIL(&beam->purge_list, b); break; } else if (APR_BUCKET_IS_METADATA(b)) { APR_BUCKET_REMOVE(b); - H2_BLIST_INSERT_TAIL(&beam->purge, b); + H2_BLIST_INSERT_TAIL(&beam->purge_list, b); } else { /* another data bucket before this one in hold. this @@ -373,7 +373,7 @@ static void h2_beam_emitted(h2_bucket_beam *beam, h2_beam_proxy *proxy) } else { /* it should be there unless we screwed up */ - ap_log_perror(APLOG_MARK, APLOG_WARNING, 0, beam->red_pool, + ap_log_perror(APLOG_MARK, APLOG_WARNING, 0, beam->send_pool, APLOGNO(03384) "h2_beam(%d-%s): emitted bucket not " "in hold, n=%d", beam->id, beam->tag, (int)proxy->n); @@ -382,7 +382,7 @@ static void h2_beam_emitted(h2_bucket_beam *beam, h2_beam_proxy *proxy) } /* notify anyone waiting on space to become available */ if (!bl.mutex) { - r_purge_reds(beam); + r_purge_sent(beam); } else if (beam->m_cond) { apr_thread_cond_broadcast(beam->m_cond); @@ -412,40 +412,38 @@ static apr_status_t beam_close(h2_bucket_beam *beam) return APR_SUCCESS; } -static void beam_set_red_pool(h2_bucket_beam *beam, apr_pool_t *pool); -static void beam_set_green_pool(h2_bucket_beam *beam, apr_pool_t *pool); +static void beam_set_send_pool(h2_bucket_beam *beam, apr_pool_t *pool); +static void beam_set_recv_pool(h2_bucket_beam *beam, apr_pool_t *pool); -static apr_status_t beam_green_cleanup(void *data) +static apr_status_t beam_recv_cleanup(void *data) { h2_bucket_beam *beam = data; - - if (beam->green) { - apr_brigade_destroy(beam->green); - beam->green = NULL; - } - beam->green_pool = NULL; + /* receiver pool has gone away, clear references */ + beam->recv_buffer = NULL; + beam->recv_pool = NULL; return APR_SUCCESS; } -static void beam_set_green_pool(h2_bucket_beam *beam, apr_pool_t *pool) +static void beam_set_recv_pool(h2_bucket_beam *beam, apr_pool_t *pool) { - if (beam->green_pool != pool) { - if (beam->green_pool) { - apr_pool_cleanup_kill(beam->green_pool, beam, beam_green_cleanup); + /* if the beam owner is the sender, monitor receiver pool lifetime */ + if (beam->owner == H2_BEAM_OWNER_SEND && beam->recv_pool != pool) { + if (beam->recv_pool) { + apr_pool_cleanup_kill(beam->recv_pool, beam, beam_recv_cleanup); } - beam->green_pool = pool; - if (beam->green_pool) { - apr_pool_pre_cleanup_register(beam->green_pool, beam, beam_green_cleanup); + beam->recv_pool = pool; + if (beam->recv_pool) { + apr_pool_pre_cleanup_register(beam->recv_pool, beam, beam_recv_cleanup); } } } -static apr_status_t beam_red_cleanup(void *data) +static apr_status_t beam_send_cleanup(void *data) { h2_bucket_beam *beam = data; - - r_purge_reds(beam); - h2_blist_cleanup(&beam->red); + /* sender has gone away, clear up all references to its memory */ + r_purge_sent(beam); + h2_blist_cleanup(&beam->send_list); report_consumption(beam, 0); while (!H2_BPROXY_LIST_EMPTY(&beam->proxies)) { h2_beam_proxy *proxy = H2_BPROXY_LIST_FIRST(&beam->proxies); @@ -453,21 +451,22 @@ static apr_status_t beam_red_cleanup(void *data) proxy->beam = NULL; proxy->bred = NULL; } - h2_blist_cleanup(&beam->purge); - h2_blist_cleanup(&beam->hold); - beam->red_pool = NULL; + h2_blist_cleanup(&beam->purge_list); + h2_blist_cleanup(&beam->hold_list); + beam->send_pool = NULL; return APR_SUCCESS; } -static void beam_set_red_pool(h2_bucket_beam *beam, apr_pool_t *pool) +static void beam_set_send_pool(h2_bucket_beam *beam, apr_pool_t *pool) { - if (beam->red_pool != pool) { - if (beam->red_pool) { - apr_pool_cleanup_kill(beam->red_pool, beam, beam_red_cleanup); + /* if the beam owner is the receiver, monitor sender pool lifetime */ + if (beam->owner == H2_BEAM_OWNER_RECV && beam->send_pool != pool) { + if (beam->send_pool) { + apr_pool_cleanup_kill(beam->send_pool, beam, beam_send_cleanup); } - beam->red_pool = pool; - if (beam->red_pool) { - apr_pool_pre_cleanup_register(beam->red_pool, beam, beam_red_cleanup); + beam->send_pool = pool; + if (beam->send_pool) { + apr_pool_pre_cleanup_register(beam->send_pool, beam, beam_send_cleanup); } } } @@ -475,17 +474,44 @@ static void beam_set_red_pool(h2_bucket_beam *beam, apr_pool_t *pool) static apr_status_t beam_cleanup(void *data) { h2_bucket_beam *beam = data; - apr_status_t status; - + apr_status_t status = APR_SUCCESS; + /* owner of the beam is going away, depending on its role, cleanup + * strategies differ. */ beam_close(beam); - if (beam->green_pool) { - apr_pool_cleanup_kill(beam->green_pool, beam, beam_green_cleanup); - status = beam_green_cleanup(beam); - } - - if (beam->red_pool) { - apr_pool_cleanup_kill(beam->red_pool, beam, beam_red_cleanup); - status = beam_red_cleanup(beam); + switch (beam->owner) { + case H2_BEAM_OWNER_SEND: + status = beam_send_cleanup(beam); + beam->recv_buffer = NULL; + beam->recv_pool = NULL; + break; + case H2_BEAM_OWNER_RECV: + if (beam->recv_buffer) { + apr_brigade_destroy(beam->recv_buffer); + } + beam->recv_buffer = NULL; + beam->recv_pool = NULL; + if (!H2_BLIST_EMPTY(&beam->send_list)) { + ap_assert(beam->send_pool); + } + if (beam->send_pool) { + /* sender has not cleaned up, its pool still lives. + * this is normal if the sender uses cleanup via a bucket + * such as the BUCKET_EOR for requests. In that case, the + * beam should have lost its mutex protection, meaning + * it is no longer used multi-threaded and we can safely + * purge all remaining sender buckets. */ + apr_pool_cleanup_kill(beam->send_pool, beam, beam_send_cleanup); + ap_assert(!beam->m_enter); + beam_send_cleanup(beam); + } + ap_assert(H2_BPROXY_LIST_EMPTY(&beam->proxies)); + ap_assert(H2_BLIST_EMPTY(&beam->send_list)); + ap_assert(H2_BLIST_EMPTY(&beam->hold_list)); + ap_assert(H2_BLIST_EMPTY(&beam->purge_list)); + break; + default: + ap_assert(NULL); + break; } return status; } @@ -498,6 +524,7 @@ apr_status_t h2_beam_destroy(h2_bucket_beam *beam) apr_status_t h2_beam_create(h2_bucket_beam **pbeam, apr_pool_t *pool, int id, const char *tag, + h2_beam_owner_t owner, apr_size_t max_buf_size) { h2_bucket_beam *beam; @@ -511,9 +538,10 @@ apr_status_t h2_beam_create(h2_bucket_beam **pbeam, apr_pool_t *pool, beam->id = id; beam->tag = tag; beam->pool = pool; - H2_BLIST_INIT(&beam->red); - H2_BLIST_INIT(&beam->hold); - H2_BLIST_INIT(&beam->purge); + beam->owner = owner; + H2_BLIST_INIT(&beam->send_list); + H2_BLIST_INIT(&beam->hold_list); + H2_BLIST_INIT(&beam->purge_list); H2_BPROXY_LIST_INIT(&beam->proxies); beam->max_buf_size = max_buf_size; apr_pool_pre_cleanup_register(pool, beam, beam_cleanup); @@ -589,8 +617,8 @@ void h2_beam_abort(h2_bucket_beam *beam) if (enter_yellow(beam, &bl) == APR_SUCCESS) { if (!beam->aborted) { beam->aborted = 1; - r_purge_reds(beam); - h2_blist_cleanup(&beam->red); + r_purge_sent(beam); + h2_blist_cleanup(&beam->send_list); report_consumption(beam, 0); } if (beam->m_cond) { @@ -605,7 +633,7 @@ apr_status_t h2_beam_close(h2_bucket_beam *beam) h2_beam_lock bl; if (enter_yellow(beam, &bl) == APR_SUCCESS) { - r_purge_reds(beam); + r_purge_sent(beam); beam_close(beam); report_consumption(beam, 0); leave_yellow(beam, &bl); @@ -620,7 +648,7 @@ apr_status_t h2_beam_wait_empty(h2_bucket_beam *beam, apr_read_type_e block) if ((status = enter_yellow(beam, &bl)) == APR_SUCCESS) { while (status == APR_SUCCESS - && !H2_BLIST_EMPTY(&beam->red) + && !H2_BLIST_EMPTY(&beam->send_list) && !H2_BPROXY_LIST_EMPTY(&beam->proxies)) { if (block == APR_NONBLOCK_READ || !bl.mutex) { status = APR_EAGAIN; @@ -643,12 +671,12 @@ static void move_to_hold(h2_bucket_beam *beam, while (red_brigade && !APR_BRIGADE_EMPTY(red_brigade)) { b = APR_BRIGADE_FIRST(red_brigade); APR_BUCKET_REMOVE(b); - H2_BLIST_INSERT_TAIL(&beam->red, b); + H2_BLIST_INSERT_TAIL(&beam->send_list, b); } } static apr_status_t append_bucket(h2_bucket_beam *beam, - apr_bucket *bred, + apr_bucket *b, apr_read_type_e block, h2_beam_lock *pbl) { @@ -657,28 +685,28 @@ static apr_status_t append_bucket(h2_bucket_beam *beam, apr_size_t space_left = 0; apr_status_t status; - if (APR_BUCKET_IS_METADATA(bred)) { - if (APR_BUCKET_IS_EOS(bred)) { + if (APR_BUCKET_IS_METADATA(b)) { + if (APR_BUCKET_IS_EOS(b)) { beam->closed = 1; } - APR_BUCKET_REMOVE(bred); - H2_BLIST_INSERT_TAIL(&beam->red, bred); + APR_BUCKET_REMOVE(b); + H2_BLIST_INSERT_TAIL(&beam->send_list, b); return APR_SUCCESS; } - else if (APR_BUCKET_IS_FILE(bred)) { + else if (APR_BUCKET_IS_FILE(b)) { /* file bucket lengths do not really count */ } else { space_left = calc_space_left(beam); - if (space_left > 0 && bred->length == ((apr_size_t)-1)) { + if (space_left > 0 && b->length == ((apr_size_t)-1)) { const char *data; - status = apr_bucket_read(bred, &data, &len, APR_BLOCK_READ); + status = apr_bucket_read(b, &data, &len, APR_BLOCK_READ); if (status != APR_SUCCESS) { return status; } } - if (space_left < bred->length) { + if (space_left < b->length) { status = r_wait_space(beam, block, pbl, &space_left); if (status != APR_SUCCESS) { return status; @@ -696,30 +724,30 @@ static apr_status_t append_bucket(h2_bucket_beam *beam, * its pool/bucket_alloc from a foreign thread and that will * corrupt. */ status = APR_ENOTIMPL; - if (APR_BUCKET_IS_TRANSIENT(bred)) { + if (APR_BUCKET_IS_TRANSIENT(b)) { /* this takes care of transient buckets and converts them * into heap ones. Other bucket types might or might not be * affected by this. */ - status = apr_bucket_setaside(bred, beam->red_pool); + status = apr_bucket_setaside(b, beam->send_pool); } - else if (APR_BUCKET_IS_HEAP(bred)) { + else if (APR_BUCKET_IS_HEAP(b)) { /* For heap buckets read from a green thread is fine. The * data will be there and live until the bucket itself is * destroyed. */ status = APR_SUCCESS; } - else if (APR_BUCKET_IS_POOL(bred)) { + else if (APR_BUCKET_IS_POOL(b)) { /* pool buckets are bastards that register at pool cleanup * to morph themselves into heap buckets. That may happen anytime, * even after the bucket data pointer has been read. So at * any time inside the green thread, the pool bucket memory * may disappear. yikes. */ - status = apr_bucket_read(bred, &data, &len, APR_BLOCK_READ); + status = apr_bucket_read(b, &data, &len, APR_BLOCK_READ); if (status == APR_SUCCESS) { - apr_bucket_heap_make(bred, data, len, NULL); + apr_bucket_heap_make(b, data, len, NULL); } } - else if (APR_BUCKET_IS_FILE(bred)) { + else if (APR_BUCKET_IS_FILE(b)) { /* For file buckets the problem is their internal readpool that * is used on the first read to allocate buffer/mmap. * Since setting aside a file bucket will de-register the @@ -729,14 +757,14 @@ static apr_status_t append_bucket(h2_bucket_beam *beam, * handles across. The use case for this is to limit the number * of open file handles and rather use a less efficient beam * transport. */ - apr_file_t *fd = ((apr_bucket_file *)bred->data)->fd; + apr_file_t *fd = ((apr_bucket_file *)b->data)->fd; int can_beam = 1; if (beam->last_beamed != fd && beam->can_beam_fn) { can_beam = beam->can_beam_fn(beam->can_beam_ctx, beam, fd); } if (can_beam) { beam->last_beamed = fd; - status = apr_bucket_setaside(bred, beam->red_pool); + status = apr_bucket_setaside(b, beam->send_pool); } /* else: enter ENOTIMPL case below */ } @@ -751,12 +779,12 @@ static apr_status_t append_bucket(h2_bucket_beam *beam, if (space_left < APR_BUCKET_BUFF_SIZE) { space_left = APR_BUCKET_BUFF_SIZE; } - if (space_left < bred->length) { - apr_bucket_split(bred, space_left); + if (space_left < b->length) { + apr_bucket_split(b, space_left); } - status = apr_bucket_read(bred, &data, &len, APR_BLOCK_READ); + status = apr_bucket_read(b, &data, &len, APR_BLOCK_READ); if (status == APR_SUCCESS) { - status = apr_bucket_setaside(bred, beam->red_pool); + status = apr_bucket_setaside(b, beam->send_pool); } } @@ -764,9 +792,9 @@ static apr_status_t append_bucket(h2_bucket_beam *beam, return status; } - APR_BUCKET_REMOVE(bred); - H2_BLIST_INSERT_TAIL(&beam->red, bred); - beam->sent_bytes += bred->length; + APR_BUCKET_REMOVE(b); + H2_BLIST_INSERT_TAIL(&beam->send_list, b); + beam->sent_bytes += b->length; return APR_SUCCESS; } @@ -775,13 +803,16 @@ apr_status_t h2_beam_send(h2_bucket_beam *beam, apr_bucket_brigade *red_brigade, apr_read_type_e block) { - apr_bucket *bred; + apr_bucket *b; apr_status_t status = APR_SUCCESS; h2_beam_lock bl; /* Called from the red thread to add buckets to the beam */ if (enter_yellow(beam, &bl) == APR_SUCCESS) { - r_purge_reds(beam); + r_purge_sent(beam); + if (red_brigade) { + beam_set_send_pool(beam, red_brigade->p); + } if (beam->aborted) { move_to_hold(beam, red_brigade); @@ -791,9 +822,8 @@ apr_status_t h2_beam_send(h2_bucket_beam *beam, int force_report = !APR_BRIGADE_EMPTY(red_brigade); while (!APR_BRIGADE_EMPTY(red_brigade) && status == APR_SUCCESS) { - bred = APR_BRIGADE_FIRST(red_brigade); - beam_set_red_pool(beam, red_brigade->p); - status = append_bucket(beam, bred, block, &bl); + b = APR_BRIGADE_FIRST(red_brigade); + status = append_bucket(beam, b, block, &bl); } report_production(beam, force_report); if (beam->m_cond) { @@ -821,19 +851,19 @@ apr_status_t h2_beam_receive(h2_bucket_beam *beam, if (enter_yellow(beam, &bl) == APR_SUCCESS) { transfer: if (beam->aborted) { - if (beam->green && !APR_BRIGADE_EMPTY(beam->green)) { - apr_brigade_cleanup(beam->green); + if (beam->recv_buffer && !APR_BRIGADE_EMPTY(beam->recv_buffer)) { + apr_brigade_cleanup(beam->recv_buffer); } status = APR_ECONNABORTED; goto leave; } /* transfer enough buckets from our green brigade, if we have one */ - beam_set_green_pool(beam, bb->p); - while (beam->green - && !APR_BRIGADE_EMPTY(beam->green) + beam_set_recv_pool(beam, bb->p); + while (beam->recv_buffer + && !APR_BRIGADE_EMPTY(beam->recv_buffer) && (readbytes <= 0 || remain >= 0)) { - bgreen = APR_BRIGADE_FIRST(beam->green); + bgreen = APR_BRIGADE_FIRST(beam->recv_buffer); if (readbytes > 0 && bgreen->length > 0 && remain <= 0) { break; } @@ -845,8 +875,8 @@ transfer: /* transfer from our red brigade, transforming red buckets to * green ones until we have enough */ - while (!H2_BLIST_EMPTY(&beam->red) && (readbytes <= 0 || remain >= 0)) { - bred = H2_BLIST_FIRST(&beam->red); + while (!H2_BLIST_EMPTY(&beam->send_list) && (readbytes <= 0 || remain >= 0)) { + bred = H2_BLIST_FIRST(&beam->send_list); bgreen = NULL; if (readbytes > 0 && bred->length > 0 && remain <= 0) { @@ -893,7 +923,7 @@ transfer: remain -= bred->length; ++transferred; APR_BUCKET_REMOVE(bred); - H2_BLIST_INSERT_TAIL(&beam->hold, bred); + H2_BLIST_INSERT_TAIL(&beam->hold_list, bred); ++transferred; continue; } @@ -910,7 +940,7 @@ transfer: /* Place the red bucket into our hold, to be destroyed when no * green bucket references it any more. */ APR_BUCKET_REMOVE(bred); - H2_BLIST_INSERT_TAIL(&beam->hold, bred); + H2_BLIST_INSERT_TAIL(&beam->hold_list, bred); beam->received_bytes += bred->length; if (bgreen) { APR_BRIGADE_INSERT_TAIL(bb, bgreen); @@ -936,17 +966,17 @@ transfer: remain -= bgreen->length; if (remain < 0) { apr_bucket_split(bgreen, bgreen->length+remain); - beam->green = apr_brigade_split_ex(bb, + beam->recv_buffer = apr_brigade_split_ex(bb, APR_BUCKET_NEXT(bgreen), - beam->green); + beam->recv_buffer); break; } } } if (beam->closed - && (!beam->green || APR_BRIGADE_EMPTY(beam->green)) - && H2_BLIST_EMPTY(&beam->red)) { + && (!beam->recv_buffer || APR_BRIGADE_EMPTY(beam->recv_buffer)) + && H2_BLIST_EMPTY(&beam->send_list)) { /* beam is closed and we have nothing more to receive */ if (!beam->close_sent) { apr_bucket *b = apr_bucket_eos_create(bb->bucket_alloc); @@ -1029,8 +1059,8 @@ apr_off_t h2_beam_get_buffered(h2_bucket_beam *beam) h2_beam_lock bl; if (enter_yellow(beam, &bl) == APR_SUCCESS) { - for (b = H2_BLIST_FIRST(&beam->red); - b != H2_BLIST_SENTINEL(&beam->red); + for (b = H2_BLIST_FIRST(&beam->send_list); + b != H2_BLIST_SENTINEL(&beam->send_list); b = APR_BUCKET_NEXT(b)) { /* should all have determinate length */ l += b->length; @@ -1047,8 +1077,8 @@ apr_off_t h2_beam_get_mem_used(h2_bucket_beam *beam) h2_beam_lock bl; if (enter_yellow(beam, &bl) == APR_SUCCESS) { - for (b = H2_BLIST_FIRST(&beam->red); - b != H2_BLIST_SENTINEL(&beam->red); + for (b = H2_BLIST_FIRST(&beam->send_list); + b != H2_BLIST_SENTINEL(&beam->send_list); b = APR_BUCKET_NEXT(b)) { if (APR_BUCKET_IS_FILE(b)) { /* do not count */ @@ -1069,8 +1099,8 @@ int h2_beam_empty(h2_bucket_beam *beam) h2_beam_lock bl; if (enter_yellow(beam, &bl) == APR_SUCCESS) { - empty = (H2_BLIST_EMPTY(&beam->red) - && (!beam->green || APR_BRIGADE_EMPTY(beam->green))); + empty = (H2_BLIST_EMPTY(&beam->send_list) + && (!beam->recv_buffer || APR_BRIGADE_EMPTY(beam->recv_buffer))); leave_yellow(beam, &bl); } return empty; diff --git a/modules/http2/h2_bucket_beam.h b/modules/http2/h2_bucket_beam.h index 4c779d1f21..fc6772be3f 100644 --- a/modules/http2/h2_bucket_beam.h +++ b/modules/http2/h2_bucket_beam.h @@ -163,6 +163,11 @@ typedef struct { typedef int h2_beam_can_beam_callback(void *ctx, h2_bucket_beam *beam, apr_file_t *file); +typedef enum { + H2_BEAM_OWNER_SEND, + H2_BEAM_OWNER_RECV +} h2_beam_owner_t; + /** * Will deny all transfer of apr_file_t across the beam and force * a data copy instead. @@ -173,13 +178,14 @@ struct h2_bucket_beam { int id; const char *tag; apr_pool_t *pool; - h2_blist red; - h2_blist hold; - h2_blist purge; - apr_bucket_brigade *green; + h2_beam_owner_t owner; + h2_blist send_list; + h2_blist hold_list; + h2_blist purge_list; + apr_bucket_brigade *recv_buffer; h2_bproxy_list proxies; - apr_pool_t *red_pool; - apr_pool_t *green_pool; + apr_pool_t *send_pool; + apr_pool_t *recv_pool; apr_size_t max_buf_size; apr_interval_time_t timeout; @@ -216,22 +222,23 @@ struct h2_bucket_beam { * mutex and will be used in multiple threads. It needs a pool allocator * that is only used inside that same mutex. * - * @param pbeam will hold the created beam on return - * @param red_pool pool usable on red side, beam lifeline + * @param pbeam will hold the created beam on return + * @param pool pool owning the beam, beam will cleanup when pool released + * @param id identifier of the beam + * @param tag tag identifying beam for logging + * @param owner if the beam is owned by the sender or receiver, e.g. if + * the pool owner is using this beam for sending or receiving * @param buffer_size maximum memory footprint of buckets buffered in beam, or * 0 for no limitation - * - * Call from the red side only. */ apr_status_t h2_beam_create(h2_bucket_beam **pbeam, - apr_pool_t *red_pool, - int id, const char *tag, + apr_pool_t *pool, + int id, const char *tag, + h2_beam_owner_t owner, apr_size_t buffer_size); /** * Destroys the beam immediately without cleanup. - * - * Call from the red side only. */ apr_status_t h2_beam_destroy(h2_bucket_beam *beam); @@ -241,10 +248,10 @@ apr_status_t h2_beam_destroy(h2_bucket_beam *beam); * All accepted buckets are removed from the given brigade. Will return with * APR_EAGAIN on non-blocking sends when not all buckets could be accepted. * - * Call from the red side only. + * Call from the sender side only. */ apr_status_t h2_beam_send(h2_bucket_beam *beam, - apr_bucket_brigade *red_buckets, + apr_bucket_brigade *bb, apr_read_type_e block); /** @@ -253,7 +260,7 @@ apr_status_t h2_beam_send(h2_bucket_beam *beam, * available or the beam has been closed. Non-blocking calls return APR_EAGAIN * if no data is available. * - * Call from the green side only. + * Call from the receiver side only. */ apr_status_t h2_beam_receive(h2_bucket_beam *beam, apr_bucket_brigade *green_buckets, @@ -262,15 +269,11 @@ apr_status_t h2_beam_receive(h2_bucket_beam *beam, /** * Determine if beam is empty. - * - * Call from red or green side. */ int h2_beam_empty(h2_bucket_beam *beam); /** * Determine if beam has handed out proxy buckets that are not destroyed. - * - * Call from red or green side. */ int h2_beam_holds_proxies(h2_bucket_beam *beam); @@ -278,14 +281,14 @@ int h2_beam_holds_proxies(h2_bucket_beam *beam); * Abort the beam. Will cleanup any buffered buckets and answer all send * and receives with APR_ECONNABORTED. * - * Call from the red side only. + * Call from the sender side only. */ void h2_beam_abort(h2_bucket_beam *beam); /** * Close the beam. Sending an EOS bucket serves the same purpose. * - * Call from the red side only. + * Call from the sender side only. */ apr_status_t h2_beam_close(h2_bucket_beam *beam); @@ -297,7 +300,7 @@ apr_status_t h2_beam_close(h2_bucket_beam *beam); * If a timeout is set on the beam, waiting might also time out and * return APR_ETIMEUP. * - * Call from the red side only. + * Call from the sender side only. */ apr_status_t h2_beam_wait_empty(h2_bucket_beam *beam, apr_read_type_e block); @@ -322,27 +325,27 @@ void h2_beam_buffer_size_set(h2_bucket_beam *beam, apr_size_t h2_beam_buffer_size_get(h2_bucket_beam *beam); /** - * Register a callback to be invoked on the red side with the - * amount of bytes that have been consumed by the red side, since the + * Register a callback to be invoked on the sender side with the + * amount of bytes that have been consumed by the receiver, since the * last callback invocation or reset. * @param beam the beam to set the callback on * @param cb the callback or NULL * @param ctx the context to use in callback invocation * - * Call from the red side, callbacks invoked on red side. + * Call from the sender side, callbacks invoked on sender side. */ void h2_beam_on_consumed(h2_bucket_beam *beam, h2_beam_io_callback *cb, void *ctx); /** - * Register a callback to be invoked on the red side with the - * amount of bytes that have been consumed by the red side, since the + * Register a callback to be invoked on the receiver side with the + * amount of bytes that have been produces by the sender, since the * last callback invocation or reset. * @param beam the beam to set the callback on * @param cb the callback or NULL * @param ctx the context to use in callback invocation * - * Call from the red side, callbacks invoked on red side. + * Call from the receiver side, callbacks invoked on receiver side. */ void h2_beam_on_produced(h2_bucket_beam *beam, h2_beam_io_callback *cb, void *ctx); diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c index d5635dd112..60f4f29adb 100644 --- a/modules/http2/h2_mplx.c +++ b/modules/http2/h2_mplx.c @@ -53,10 +53,10 @@ static void h2_beam_log(h2_bucket_beam *beam, int id, const char *msg, apr_size_t off = 0; off += apr_snprintf(buffer+off, H2_ALEN(buffer)-off, "cl=%d, ", beam->closed); - off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "red", ", ", &beam->red); - off += h2_util_bb_print(buffer+off, H2_ALEN(buffer)-off, "green", ", ", beam->green); - off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "hold", ", ", &beam->hold); - off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "purge", "", &beam->purge); + off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "red", ", ", &beam->send_list); + off += h2_util_bb_print(buffer+off, H2_ALEN(buffer)-off, "green", ", ", beam->recv_buffer); + off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "hold", ", ", &beam->hold_list); + off += h2_util_bl_print(buffer+off, H2_ALEN(buffer)-off, "purge", "", &beam->purge_list); ap_log_cerror(APLOG_MARK, level, 0, c, "beam(%ld-%d): %s %s", c->id, id, msg, buffer); @@ -1039,6 +1039,8 @@ static void task_done(h2_mplx *m, h2_task *task, h2_req_engine *ngn) * called from a worker thread and freeing memory pools * is only safe in the only thread using it (and its * parent pool / allocator) */ + h2_beam_on_consumed(stream->output, NULL, NULL); + h2_beam_mutex_set(stream->output, NULL, NULL, NULL); h2_ihash_remove(m->shold, stream->id); h2_ihash_add(m->spurge, stream); } diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c index 3f177cf54e..d3dd50cb65 100644 --- a/modules/http2/h2_stream.c +++ b/modules/http2/h2_stream.c @@ -201,8 +201,8 @@ h2_stream *h2_stream_open(int id, apr_pool_t *pool, h2_session *session, stream->pool = pool; stream->session = session; - h2_beam_create(&stream->input, pool, id, "input", 0); - h2_beam_create(&stream->output, pool, id, "output", 0); + h2_beam_create(&stream->input, pool, id, "input", H2_BEAM_OWNER_SEND, 0); + h2_beam_create(&stream->output, pool, id, "output", H2_BEAM_OWNER_RECV, 0); set_state(stream, H2_STREAM_ST_OPEN); apr_pool_cleanup_register(pool, stream, stream_pool_cleanup, |