*) mod_http2: fixes 2 regressions in server limit handling.

change-entries/h2_pr65731_issue212.txt

@@ -0,0 +1,14 @@
+  *) mod_http2: fixes 2 regressions in server limit handling.
+     1. When reaching server limits, such as MaxRequestsPerChild, the
+        HTTP/2 connection send a GOAWAY frame much too early on new
+        connections, leading to invalid protocol state and a client
+        failing the request. See PR65731.
+        The module now initializes the HTTP/2 protocol correctly and
+        allows the client to submit one request before the shutdown
+        via a GOAWAY frame is being announced.
+     2. A regression in v1.15.24 was fixed that could lead to httpd
+        child processes not being terminated on a graceful reload or
+        when reaching MaxConnectionsPerChild. When unprocessed h2
+        requests were queued at the time, these could stall.
+        See <https://github.com/icing/mod_h2/issues/212>.
+     [Stefan Eissing]

modules/http2/h2_session.c

@@ -275,7 +275,7 @@ static int on_begin_headers_cb(nghttp2_session *ngh2,
                                const nghttp2_frame *frame, void *userp)
 {
     h2_session *session = (h2_session *)userp;
-    h2_stream *s;
+    h2_stream *s = NULL;
 
     /* We may see HEADERs at the start of a stream or after all DATA
      * streams to carry trailers. */
@@ -284,7 +284,7 @@ static int on_begin_headers_cb(nghttp2_session *ngh2,
     if (s) {
         /* nop */
     }
-    else {
+    else if (session->local.accepting) {
         s = h2_session_open_stream(userp, frame->hd.stream_id, 0);
     }
     return s? 0 : NGHTTP2_ERR_START_STREAM_NOT_ALLOWED;
@@ -2115,7 +2115,16 @@ apr_status_t h2_session_process(h2_session *session, int async)
         now = apr_time_now();
         session->have_read = session->have_written = 0;
 
-        if (session->local.accepting 
+        /* PR65731: we may get a new connection to process while the
+         * MPM already is stopping. For example due to having reached
+         * MaxRequestsPerChild limit.
+         * Since this is supposed to handle things gracefully, we need to:
+         * a) fully initialize the session before GOAWAYing
+         * b) give the client the chance to submit at least one request
+         */
+        if (session->state != H2_SESSION_ST_INIT /* no longer intializing */
+            && session->local.accepted_max > 0   /* have gotten at least one stream */
+            && session->local.accepting          /* have not already locally shut down */
             && !ap_mpm_query(AP_MPMQ_MPM_STATE, &mpm_state)) {
             if (mpm_state == AP_MPMQ_STOPPING) {
                 dispatch_event(session, H2_SESSION_EV_MPM_STOPPING, 0, NULL);

modules/http2/h2_version.h

@@ -27,15 +27,15 @@
  * @macro
  * Version number of the http2 module as c string
  */
-#define MOD_HTTP2_VERSION "1.15.24"
+#define MOD_HTTP2_VERSION "1.15.26"
 
 /**
  * @macro
  * Numerical representation of the version number of the http2 module
  * release. This is a 24 bit number with 8 bits for major number, 8 bits
  * for minor and 8 bits for patch. Version 1.2.3 becomes 0x010203.
  */
-#define MOD_HTTP2_VERSION_NUM 0x010f18
+#define MOD_HTTP2_VERSION_NUM 0x010f1a
 
 
 #endif /* mod_h2_h2_version_h */

modules/http2/h2_workers.c

@@ -479,8 +479,6 @@ apr_status_t h2_workers_unregister(h2_workers *workers, struct h2_mplx *m)
 void h2_workers_graceful_shutdown(h2_workers *workers)
 {
     workers->shutdown = 1;
-    workers->min_workers = 1;
     workers->max_idle_duration = apr_time_from_sec(1);
-    h2_fifo_term(workers->mplxs);
     wake_non_essential_workers(workers);
 }