Merge r1888087 from trunk

   * mod_http2: Fixed a race condition that could lead to streams being
     aborted (RST to the client), although a response had been produced

Submitted by: icing
Reviewed by: icing, rpluem, covener
Backported by: jailletc36

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1888922 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/changes-entries/h2_close_race.txt b/changes-entries/h2_close_race.txt
new file mode 100644 (file)
index 0000000..6ac04da
--- /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 bc09ff1a1349fcecb0aa297a9459a3bf6a9898a6..be535e137fe1829b285220365206bd1f3546c017 100644 (file)
--- a/modules/http2/h2_bucket_beam.c
+++ b/modules/http2/h2_bucket_beam.c
@@ -945,7 +945,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;
@@ -953,7 +954,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) {
@@ -1127,7 +1128,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 1a6418cefc3e84f55cbcc6c040b59ea55fa80f68..6cc1adecb8400ceef26fe10840031b558abe2b2c 100644 (file)
--- 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 dce1fb45b4be0643cb38bc6405bafa8e0bbdc286..b717888ad966591afbbce050db685c6a332f4761 100644 (file)
--- 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 0b59cd521004169241df1c83f03441320e06fa38..74b814747197a2e8e3a988152ad77d958aadee79 100644 (file)
--- 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;