diff --git a/changes-entries/h2_close_race.txt b/changes-entries/h2_close_race.txt new file mode 100644 index 0000000000..6ac04da97b --- /dev/null +++ b/changes-entries/h2_close_race.txt @@ -0,0 +1,3 @@ + *) mod_http2: Fixed a race condition that could lead to streams being + aborted (RST to the client), although a response had been produced. + [Stefan Eissing] diff --git a/modules/http2/h2_bucket_beam.c b/modules/http2/h2_bucket_beam.c index b857afcddc..3e09fcd017 100644 --- a/modules/http2/h2_bucket_beam.c +++ b/modules/http2/h2_bucket_beam.c @@ -948,7 +948,8 @@ apr_status_t h2_beam_send(h2_bucket_beam *beam, apr_status_t h2_beam_receive(h2_bucket_beam *beam, apr_bucket_brigade *bb, apr_read_type_e block, - apr_off_t readbytes) + apr_off_t readbytes, + int *pclosed) { h2_beam_lock bl; apr_bucket *bsender, *brecv, *ng; @@ -956,7 +957,7 @@ apr_status_t h2_beam_receive(h2_bucket_beam *beam, apr_status_t status = APR_SUCCESS; apr_off_t remain; int transferred_buckets = 0; - + /* Called from the receiver thread to take buckets from the beam */ if (enter_yellow(beam, &bl) == APR_SUCCESS) { if (readbytes <= 0) { @@ -1130,7 +1131,8 @@ transfer: } goto transfer; } -leave: +leave: + if (pclosed) *pclosed = beam->closed? 1 : 0; leave_yellow(beam, &bl); } return status; diff --git a/modules/http2/h2_bucket_beam.h b/modules/http2/h2_bucket_beam.h index 1a6418cefc..6cc1adecb8 100644 --- a/modules/http2/h2_bucket_beam.h +++ b/modules/http2/h2_bucket_beam.h @@ -258,11 +258,15 @@ void h2_beam_send_from(h2_bucket_beam *beam, apr_pool_t *p); * if no data is available. * * Call from the receiver side only. + * @param pclosed on return != 0 iff the beam has been closed by the sender. It + * may still hold untransfered data. Maybe NULL if the caller is + * not interested in this. */ apr_status_t h2_beam_receive(h2_bucket_beam *beam, apr_bucket_brigade *green_buckets, apr_read_type_e block, - apr_off_t readbytes); + apr_off_t readbytes, + int *pclosed); /** * Determine if beam is empty. diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c index dce1fb45b4..b717888ad9 100644 --- a/modules/http2/h2_stream.c +++ b/modules/http2/h2_stream.c @@ -901,7 +901,7 @@ apr_status_t h2_stream_out_prepare(h2_stream *stream, apr_off_t *plen, apr_status_t status = APR_SUCCESS; apr_off_t requested, missing, max_chunk = H2_DATA_CHUNK_SIZE; conn_rec *c; - int complete; + int complete, was_closed = 0; ap_assert(stream); @@ -950,9 +950,11 @@ apr_status_t h2_stream_out_prepare(h2_stream *stream, apr_off_t *plen, if (stream->output) { H2_STREAM_OUT_LOG(APLOG_TRACE2, stream, "pre"); - rv = h2_beam_receive(stream->output, stream->out_buffer, - APR_NONBLOCK_READ, stream->max_mem - *plen); + h2_beam_log(stream->output, c, APLOG_TRACE2, "pre read output"); + rv = h2_beam_receive(stream->output, stream->out_buffer, + APR_NONBLOCK_READ, stream->max_mem - *plen, &was_closed); H2_STREAM_OUT_LOG(APLOG_TRACE2, stream, "post"); + h2_beam_log(stream->output, c, APLOG_TRACE2, "post read output"); } if (rv == APR_SUCCESS) { @@ -982,7 +984,7 @@ apr_status_t h2_stream_out_prepare(h2_stream *stream, apr_off_t *plen, (long)*plen, *peos); } else { - status = (stream->output && h2_beam_is_closed(stream->output))? APR_EOF : APR_EAGAIN; + status = was_closed? APR_EOF : APR_EAGAIN; ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, H2_STRM_MSG(stream, "prepare, no data")); } diff --git a/modules/http2/h2_task.c b/modules/http2/h2_task.c index 4edaf92cda..399c40b60f 100644 --- a/modules/http2/h2_task.c +++ b/modules/http2/h2_task.c @@ -266,7 +266,7 @@ static apr_status_t h2_filter_secondary_in(ap_filter_t* f, } if (task->input.beam) { status = h2_beam_receive(task->input.beam, task->input.bb, block, - 128*1024); + 128*1024, NULL); } else { status = APR_EOF;