On the trunk:

mod_http2: facilitate slave connection reuse by comforting ap_check_pipeline.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1782944 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 5f8cba3fe3d12ebe2a830e1e1d1dc33c89a428e6..58711575e75e6b918610a2890b71756a5677e2f2 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_http2: comforts ap_check_pipeline() on slave connections
+     to facilitate reuse (see https://github.com/icing/mod_h2/issues/128).
+     [Stefan Eissing, reported by Armin Abfalterer]
+     
   *) mod_http2: http/2 streams now with state handling/transitions as defined
      in RFC7540. Stream cleanup/connection shutdown reworked to become easier
      to understand/maintain/debug. Added many asserts on state and cleanup 

diff --git a/modules/http2/h2_bucket_beam.c b/modules/http2/h2_bucket_beam.c
index 6410bcdd1828c13b0485bff1ca07472c5c8f150a..ca57ee959079a3c6eed01ee78519c32d2d355f09 100644 (file)
--- a/modules/http2/h2_bucket_beam.c
+++ b/modules/http2/h2_bucket_beam.c
@@ -191,36 +191,6 @@ static apr_bucket *h2_beam_bucket(h2_bucket_beam *beam,
 }
 
 
-static apr_size_t h2_util_bl_print(char *buffer, apr_size_t bmax, 
-                                   const char *tag, const char *sep, 
-                                   h2_blist *bl)
-{
-    apr_size_t off = 0;
-    const char *sp = "";
-    apr_bucket *b;
-    
-    if (bl) {
-        memset(buffer, 0, bmax--);
-        off += apr_snprintf(buffer+off, bmax-off, "%s(", tag);
-        for (b = H2_BLIST_FIRST(bl); 
-             (bmax > off) && (b != H2_BLIST_SENTINEL(bl));
-             b = APR_BUCKET_NEXT(b)) {
-            
-            off += h2_util_bucket_print(buffer+off, bmax-off, b, sp);
-            sp = " ";
-        }
-        if (bmax > off) {
-            off += apr_snprintf(buffer+off, bmax-off, ")%s", sep);
-        }
-    }
-    else {
-        off += apr_snprintf(buffer+off, bmax-off, "%s(null)%s", tag, sep);
-    }
-    return off;
-}
-
-
-
 /*******************************************************************************
  * bucket beam that can transport buckets across threads
  ******************************************************************************/
@@ -1235,26 +1205,12 @@ int h2_beam_report_consumption(h2_bucket_beam *beam)
 
 void h2_beam_log(h2_bucket_beam *beam, conn_rec *c, int level, const char *msg)
 {
-    if (0 && beam && APLOG_C_IS_LEVEL(c,level)) {
-        char buffer[2048];
-        apr_size_t blen = sizeof(buffer)/sizeof(buffer[0]) - 1;
-        apr_size_t off = 0;
-        
-        buffer[0] = 0;
-        off += apr_snprintf(buffer+off, blen-off, "cl=%d, ", beam->closed);
-        off += h2_util_bl_print(buffer+off, blen-off, "to_send", ", ", &beam->send_list);
-        if (blen > off) {
-            off += h2_util_bb_print(buffer+off, blen-off, "recv_buffer", ", ", beam->recv_buffer);
-            if (blen > off) {
-                off += h2_util_bl_print(buffer+off, blen-off, "hold", ", ", &beam->hold_list);
-                if (blen > off) {
-                    off += h2_util_bl_print(buffer+off, blen-off, "purge", "", &beam->purge_list);
-                }
-            }
-        }
-        buffer[blen-1] = 0;
-        ap_log_cerror(APLOG_MARK, level, 0, c, "beam(%ld-%d,%s): %s %s", 
-                      c->id, beam->id, beam->tag, msg, buffer);
+    if (beam && APLOG_C_IS_LEVEL(c,level)) {
+        ap_log_cerror(APLOG_MARK, level, 0, c, 
+                      "beam(%ld-%d,%s,closed=%d,aborted=%d,empty=%d,buf=%ld): %s", 
+                      c->id, beam->id, beam->tag, beam->closed, beam->aborted, 
+                      h2_beam_empty(beam), (long)h2_beam_get_buffered(beam),
+                      msg);
     }
 }
 

diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c
index 164abb2e474141d00c5cd7649d4362dd297c320e..ed71b8607758ea457d8dde887da4aafbf29e9e92 100644 (file)
--- a/modules/http2/h2_mplx.c
+++ b/modules/http2/h2_mplx.c
@@ -328,19 +328,20 @@ static void task_destroy(h2_mplx *m, h2_task *task)
     conn_rec *slave = NULL;
     int reuse_slave = 0;
     
-    h2_beam_log(task->output.beam, m->c, APLOG_DEBUG, 
-                  APLOGNO(03385) "h2_task_destroy");
-    
     slave = task->c;
     reuse_slave = ((m->spare_slaves->nelts < m->spare_slaves->nalloc)
                    && !task->rst_error);
     
     if (slave) {
         if (reuse_slave && slave->keepalive == AP_CONN_KEEPALIVE) {
+            h2_beam_log(task->output.beam, m->c, APLOG_DEBUG, 
+                        APLOGNO(03385) "h2_task_destroy, reuse slave");    
             h2_task_destroy(task);
             APR_ARRAY_PUSH(m->spare_slaves, conn_rec*) = slave;
         }
         else {
+            h2_beam_log(task->output.beam, m->c, APLOG_TRACE1, 
+                        "h2_task_destroy, destroy slave");    
             slave->sbh = NULL;
             h2_slave_destroy(slave);
         }

diff --git a/modules/http2/h2_task.c b/modules/http2/h2_task.c
index 8aca24ba093c6b8ef0ae3f1f6ce8da6e4f34b7a7..23439e3c1bee722700328d1181927d9bf98daa92 100644 (file)
--- a/modules/http2/h2_task.c
+++ b/modules/http2/h2_task.c
@@ -254,9 +254,6 @@ static apr_status_t h2_filter_slave_in(ap_filter_t* f,
         ap_log_cerror(APLOG_MARK, APLOG_TRACE1, status, f->c,
                       "h2_slave_in(%s): get more data from mplx, block=%d, "
                       "readbytes=%ld", task->id, block, (long)readbytes);
-        
-        /* Override the block mode we get called with depending on the input's
-         * setting. */
         if (task->input.beam) {
             status = h2_beam_receive(task->input.beam, task->input.bb, block, 
                                      H2MIN(readbytes, 32*1024));
@@ -289,10 +286,11 @@ static apr_status_t h2_filter_slave_in(ap_filter_t* f,
         }
     }
     
-    /* Inspect the buckets received, detect EOS and apply
-     * chunked encoding if necessary */
+    /* Nothing there, no more data to get. Return APR_EAGAIN on
+     * speculative reads, this is ap_check_pipeline()'s trick to
+     * see if the connection needs closing. */
     if (status == APR_EOF && APR_BRIGADE_EMPTY(task->input.bb)) {
-        return APR_EOF;
+        return (mode == AP_MODE_SPECULATIVE)? APR_EAGAIN : APR_EOF;
     }
 
     h2_util_bb_log(f->c, task->stream_id, APLOG_TRACE2,