Merge of 1761479,1761548,1762703,1763158 from trunk

mod_http2: rewrite of how responses and trailers are transferred between
     master and slave connection. Reduction of internal states for tasks
     and streams, stability. Heuristic id generation for slave connections
     to better keep promise of connection ids unique at given point int time.
     Fix for mod_cgid interop in high load situtations. 
     Fix for handling of incoming trailers when no request body is sent.



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

CHANGES

@@ -2,8 +2,16 @@
 
 Changes with Apache 2.4.24
 
+  *) mod_http2: rewrite of how responses and trailers are transferred between
+     master and slave connection. Reduction of internal states for tasks
+     and streams, stability. Heuristic id generation for slave connections
+     to better keep promise of connection ids unique at given point int time.
+     Fix for mod_cgid interop in high load situtations. 
+     Fix for handling of incoming trailers when no request body is sent.
+     [Stefan Eissing]
+
   *) mod_http2: fix suspended handling for streams. Output could become
-     blocked in rare cases.
+     blocked in rare cases. [Stefan Eissing]
 
   *) mpm_winnt: Prevent a denial of service when the 'data' AcceptFilter is in
      use by replacing it with the 'connect' filter. PR 59970. [Jacob Champion]

CMakeLists.txt

@@ -387,7 +387,7 @@ SET(mod_http2_extra_sources
   modules/http2/h2_from_h1.c         modules/http2/h2_h2.c
   modules/http2/h2_bucket_beam.c
   modules/http2/h2_mplx.c            modules/http2/h2_push.c
-  modules/http2/h2_request.c         modules/http2/h2_response.c
+  modules/http2/h2_request.c         modules/http2/h2_headers.c
   modules/http2/h2_session.c         modules/http2/h2_stream.c 
   modules/http2/h2_switch.c          modules/http2/h2_ngn_shed.c 
   modules/http2/h2_task.c            modules/http2/h2_util.c

modules/http2/NWGNUmod_http2

@@ -199,7 +199,7 @@ FILES_nlm_objs = \
 	$(OBJDIR)/h2_ngn_shed.o \
 	$(OBJDIR)/h2_push.o \
 	$(OBJDIR)/h2_request.o \
-	$(OBJDIR)/h2_response.o \
+	$(OBJDIR)/h2_headers.o \
 	$(OBJDIR)/h2_session.o \
 	$(OBJDIR)/h2_stream.o \
 	$(OBJDIR)/h2_switch.o \

modules/http2/config2.m4

@@ -30,11 +30,11 @@ h2_ctx.lo dnl
 h2_filter.lo dnl
 h2_from_h1.lo dnl
 h2_h2.lo dnl
+h2_headers.lo dnl
 h2_mplx.lo dnl
 h2_ngn_shed.lo dnl
 h2_push.lo dnl
 h2_request.lo dnl
-h2_response.lo dnl
 h2_session.lo dnl
 h2_stream.lo dnl
 h2_switch.lo dnl

modules/http2/h2.h

@@ -47,6 +47,9 @@ extern const char *H2_MAGIC_TOKEN;
 #define H2_HEADER_PATH_LEN   5
 #define H2_CRLF             "\r\n"
 
+/* Max data size to write so it fits inside a TLS record */
+#define H2_DATA_CHUNK_SIZE          ((16*1024) - 100 - 9) 
+
 /* Maximum number of padding bytes in a frame, rfc7540 */
 #define H2_MAX_PADLEN               256
 /* Initial default window size, RFC 7540 ch. 6.5.2 */
@@ -115,39 +118,25 @@ typedef struct h2_session_props {
 typedef struct h2_request h2_request;
 
 struct h2_request {
-    int id;             /* stream id */
-    int initiated_on;   /* initiating stream id (PUSH) or 0 */
-
     const char *method; /* pseudo header values, see ch. 8.1.2.3 */
     const char *scheme;
     const char *authority;
     const char *path;
 
     apr_table_t *headers;
-    apr_table_t *trailers;
 
     apr_time_t request_time;
-    apr_off_t content_length;
 
-    unsigned int chunked : 1; /* iff requst body needs to be forwarded as chunked */
-    unsigned int eoh     : 1; /* iff end-of-headers has been seen and request is complete */
-    unsigned int body    : 1; /* iff this request has a body */
+    unsigned int chunked : 1;   /* iff requst body needs to be forwarded as chunked */
     unsigned int serialize : 1; /* iff this request is written in HTTP/1.1 serialization */
-    unsigned int push_policy; /* which push policy to use for this request */
 };
 
-typedef struct h2_response h2_response;
+typedef struct h2_headers h2_headers;
 
-struct h2_response {
-    int         stream_id;
-    int         rst_error;
-    int         http_status;
-    apr_off_t   content_length;
+struct h2_headers {
+    int         status;
     apr_table_t *headers;
-    apr_table_t *trailers;
-    struct h2_response *next;
-
-    const char  *sos_filter;
+    apr_table_t *notes;
 };
 
 typedef apr_status_t h2_io_data_cb(void *ctx, const char *data, apr_off_t len);
@@ -156,7 +145,7 @@ typedef int h2_stream_pri_cmp(int stream_id1, int stream_id2, void *ctx);
 
 /* Note key to attach connection task id to conn_rec/request_rec instances */
 
-#define H2_TASK_ID_NOTE     "http2-task-id"
-
+#define H2_TASK_ID_NOTE         "http2-task-id"
+#define H2_FILTER_DEBUG_NOTE    "http2-debug"
 
 #endif /* defined(__mod_h2__h2__) */

modules/http2/h2_bucket_beam.c

@@ -21,6 +21,7 @@
 #include <apr_thread_cond.h>
 
 #include <httpd.h>
+#include <http_protocol.h>
 #include <http_log.h>
 
 #include "h2_private.h"
@@ -170,6 +171,14 @@ const apr_bucket_type_t h2_bucket_type_beam = {
  * h2_blist, a brigade without allocations
  ******************************************************************************/
 
+APR_HOOK_STRUCT(
+            APR_HOOK_LINK(beam_bucket)
+)
+AP_IMPLEMENT_HOOK_RUN_FIRST(apr_bucket *, beam_bucket,
+                            (h2_bucket_beam *beam, apr_bucket_brigade *dest,
+                             const apr_bucket *src),
+                            (beam, dest, src), NULL)
+
 apr_size_t h2_util_bl_print(char *buffer, apr_size_t bmax, 
                             const char *tag, const char *sep, 
                             h2_blist *bl)
@@ -245,9 +254,9 @@ static void report_production(h2_bucket_beam *beam, int force)
     }
 }
 
-static apr_off_t calc_buffered(h2_bucket_beam *beam)
+static apr_size_t calc_buffered(h2_bucket_beam *beam)
 {
-    apr_off_t len = 0;
+    apr_size_t len = 0;
     apr_bucket *b;
     for (b = H2_BLIST_FIRST(&beam->red); 
          b != H2_BLIST_SENTINEL(&beam->red);
@@ -296,7 +305,7 @@ static apr_status_t wait_cond(h2_bucket_beam *beam, apr_thread_mutex_t *lock)
 }
 
 static apr_status_t r_wait_space(h2_bucket_beam *beam, apr_read_type_e block,
-                                 h2_beam_lock *pbl, apr_off_t *premain) 
+                                 h2_beam_lock *pbl, apr_size_t *premain) 
 {
     *premain = calc_space_left(beam);
     while (!beam->aborted && *premain <= 0 
@@ -428,7 +437,7 @@ apr_status_t h2_beam_destroy(h2_bucket_beam *beam)
 }
 
 apr_status_t h2_beam_create(h2_bucket_beam **pbeam, apr_pool_t *red_pool, 
-                            int id, const char *tag, 
+                            apr_uint32_t id, const char *tag, 
                             apr_size_t max_buf_size)
 {
     h2_bucket_beam *beam;
@@ -518,10 +527,12 @@ void h2_beam_abort(h2_bucket_beam *beam)
     h2_beam_lock bl;
 
     if (enter_yellow(beam, &bl) == APR_SUCCESS) {
-        r_purge_reds(beam);
-        h2_blist_cleanup(&beam->red);
-        beam->aborted = 1;
-        report_consumption(beam, 0);
+        if (!beam->aborted) {
+            beam->aborted = 1;
+            r_purge_reds(beam);
+            h2_blist_cleanup(&beam->red);
+            report_consumption(beam, 0);
+        }
         if (beam->m_cond) {
             apr_thread_cond_broadcast(beam->m_cond);
         }
@@ -585,7 +596,7 @@ static apr_status_t append_bucket(h2_bucket_beam *beam,
 {
     const char *data;
     apr_size_t len;
-    apr_off_t space_left = 0;
+    apr_size_t space_left = 0;
     apr_status_t status;
 
     if (APR_BUCKET_IS_METADATA(bred)) {
@@ -792,8 +803,10 @@ apr_status_t h2_beam_receive(h2_bucket_beam *beam,
                 else if (APR_BUCKET_IS_FLUSH(bred)) {
                     bgreen = apr_bucket_flush_create(bb->bucket_alloc);
                 }
-                else {
-                    /* put red into hold, no green sent out */
+                else if (AP_BUCKET_IS_ERROR(bred)) {
+                    ap_bucket_error *eb = (ap_bucket_error *)bred;
+                    bgreen = ap_bucket_error_create(eb->status, eb->data,
+                                                    bb->p, bb->bucket_alloc);
                 }
             }
             else if (APR_BUCKET_IS_FILE(bred)) {
@@ -846,6 +859,14 @@ apr_status_t h2_beam_receive(h2_bucket_beam *beam,
                 remain -= bgreen->length;
                 ++transferred;
             }
+            else {
+                bgreen = ap_run_beam_bucket(beam, bb, bred);
+                while (bgreen && bgreen != APR_BRIGADE_SENTINEL(bb)) {
+                    ++transferred;
+                    remain -= bgreen->length;
+                    bgreen = APR_BUCKET_NEXT(bgreen);
+                }
+            }
         }
 
         if (readbytes > 0 && remain < 0) {

modules/http2/h2_bucket_beam.h

@@ -170,7 +170,7 @@ typedef int h2_beam_can_beam_callback(void *ctx, h2_bucket_beam *beam,
 int h2_beam_no_files(void *ctx, h2_bucket_beam *beam, apr_file_t *file);
 
 struct h2_bucket_beam {
-    int id;
+    apr_uint32_t id;
     const char *tag;
     h2_blist red;
     h2_blist hold;
@@ -223,7 +223,7 @@ struct h2_bucket_beam {
  */
 apr_status_t h2_beam_create(h2_bucket_beam **pbeam,
                             apr_pool_t *red_pool, 
-                            int id, const char *tag, 
+                            apr_uint32_t id, const char *tag, 
                             apr_size_t buffer_size);
 
 /**
@@ -373,4 +373,8 @@ int h2_beam_was_received(h2_bucket_beam *beam);
 
 apr_size_t h2_beam_get_files_beamed(h2_bucket_beam *beam);
 
+AP_DECLARE_HOOK(apr_bucket *, beam_bucket,
+                (h2_bucket_beam *beam, apr_bucket_brigade *dest,
+                 const apr_bucket *src))
+
 #endif /* h2_bucket_beam_h */

modules/http2/h2_conn.c

@@ -14,6 +14,7 @@
  */
 
 #include <assert.h>
+#include <apr_strings.h>
 
 #include <ap_mpm.h>
 
@@ -240,12 +241,13 @@ apr_status_t h2_conn_pre_close(struct h2_ctx *ctx, conn_rec *c)
     return status;
 }
 
-conn_rec *h2_slave_create(conn_rec *master, apr_pool_t *parent,
-                          apr_allocator_t *allocator)
+conn_rec *h2_slave_create(conn_rec *master, apr_uint32_t slave_id, 
+                          apr_pool_t *parent, apr_allocator_t *allocator)
 {
     apr_pool_t *pool;
     conn_rec *c;
     void *cfg;
+    unsigned long l;
 
     AP_DEBUG_ASSERT(master);
     ap_log_cerror(APLOG_MARK, APLOG_TRACE3, 0, master,
@@ -271,8 +273,29 @@ conn_rec *h2_slave_create(conn_rec *master, apr_pool_t *parent,
     }
 
     memcpy(c, master, sizeof(conn_rec));
-
-    /* Replace these */
+
+    /* Each conn_rec->id is supposed to be unique at a point in time. Since
+     * some modules (and maybe external code) uses this id as an identifier
+     * for the request_rec they handle, it needs to be unique for slave 
+     * connections also.
+     * The connection id is generated by the MPM and most MPMs use the formula
+     *    id := (child_num * max_threads) + thread_num
+     * which means that there is a maximum id of about
+     *    idmax := max_child_count * max_threads
+     * If we assume 2024 child processes with 2048 threads max, we get
+     *    idmax ~= 2024 * 2048 = 2 ** 22
+     * On 32 bit systems, we have not much space left, but on 64 bit systems
+     * (and higher?) we can use the upper 32 bits without fear of collision.
+     * 32 bits is just what we need, since a connection can only handle so
+     * many streams. 
+     */
+    l = master->id;
+    if (sizeof(long) >= 8 && l < APR_UINT32_MAX) {
+        c->id = l|(((unsigned long)slave_id) << 32);
+    }
+    else {
+        c->id = l^(~slave_id);
+    }
     c->master                 = master;
     c->pool                   = pool;   
     c->conn_config            = ap_create_conn_config(pool);
@@ -284,7 +307,8 @@ conn_rec *h2_slave_create(conn_rec *master, apr_pool_t *parent,
     c->data_in_output_filters = 0;
     c->clogging_input_filters = 1;
     c->log                    = NULL;
-    c->log_id                 = NULL;
+    c->log_id                 = apr_psprintf(pool, "%ld-%d", 
+                                             master->id, slave_id);
     /* Simulate that we had already a request on this connection. */
     c->keepalives             = 1;
     /* We cannot install the master connection socket on the slaves, as
@@ -304,6 +328,9 @@ conn_rec *h2_slave_create(conn_rec *master, apr_pool_t *parent,
         ap_set_module_config(c->conn_config, h2_conn_mpm_module(), cfg);
     }
 
+    ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, 
+                  "h2_task: creating conn, master=%ld, sid=%ld, logid=%s", 
+                  master->id, c->id, c->log_id);
     return c;
 }
 

modules/http2/h2_conn.h

@@ -66,8 +66,8 @@ typedef enum {
 h2_mpm_type_t h2_conn_mpm_type(void);
 
 
-conn_rec *h2_slave_create(conn_rec *master, apr_pool_t *parent,
-                          apr_allocator_t *allocator);
+conn_rec *h2_slave_create(conn_rec *master, apr_uint32_t slave_id, 
+                          apr_pool_t *parent, apr_allocator_t *allocator);
 void h2_slave_destroy(conn_rec *slave, apr_allocator_t **pallocator);
 
 apr_status_t h2_slave_run_pre_connection(conn_rec *slave, apr_socket_t *csd);