Merge of /httpd/httpd/trunk:r1912999

 * mod_http2: improved early cleanup of streams.

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

diff --git a/changes-entries/h2_cleanup.txt b/changes-entries/h2_cleanup.txt
new file mode 100644 (file)
index 0000000..5366b4a
--- /dev/null
+++ b/changes-entries/h2_cleanup.txt
@@ -0,0 +1,2 @@
+ * mod_http2: improved early cleanup of streams.
+   [Stefan Eissing]

diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c
index 4637a5f66efd50156af5a6a2fa0d716f03e529fe..2aeea42b5df72481c8a2d9c1d9826e3198ac4ab5 100644 (file)
--- a/modules/http2/h2_mplx.c
+++ b/modules/http2/h2_mplx.c
@@ -1119,14 +1119,32 @@ static int reset_is_acceptable(h2_stream *stream)
     return 1; /* otherwise, be forgiving */
 }
 
-apr_status_t h2_mplx_c1_client_rst(h2_mplx *m, int stream_id)
+apr_status_t h2_mplx_c1_client_rst(h2_mplx *m, int stream_id, h2_stream *stream)
 {
-    h2_stream *stream;
     apr_status_t status = APR_SUCCESS;
+    int registered;
 
     H2_MPLX_ENTER_ALWAYS(m);
-    stream = h2_ihash_get(m->streams, stream_id);
-    if (stream && !reset_is_acceptable(stream)) {
+    registered = (h2_ihash_get(m->streams, stream_id) != NULL);
+    if (!stream) {
+      /* a RST might arrive so late, we have already forgotten
+       * about it. Seems ok. */
+      ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c1,
+                    H2_MPLX_MSG(m, "RST on unknown stream %d"), stream_id);
+      AP_DEBUG_ASSERT(!registered);
+    }
+    else if (!registered) {
+      /* a RST on a stream that mplx has not been told about, but
+       * which the session knows. Very early and annoying. */
+      ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, m->c1,
+                    H2_STRM_MSG(stream, "very early RST, drop"));
+      h2_stream_set_monitor(stream, NULL);
+      h2_stream_rst(stream, H2_ERR_STREAM_CLOSED);
+      h2_stream_dispatch(stream, H2_SEV_EOS_SENT);
+      m_stream_cleanup(m, stream);
+      m_be_annoyed(m);
+    }
+    else if (!reset_is_acceptable(stream)) {
         m_be_annoyed(m);
     }
     H2_MPLX_LEAVE(m);

diff --git a/modules/http2/h2_mplx.h b/modules/http2/h2_mplx.h
index a2e73d9d7c3855c9f978a8c7c84757c16a087594..860f916039785573247938950ffc3599a44507b6 100644 (file)
--- a/modules/http2/h2_mplx.h
+++ b/modules/http2/h2_mplx.h
@@ -201,7 +201,8 @@ int h2_mplx_c1_all_streams_want_send_data(h2_mplx *m);
  * any processing going on and remove from processing
  * queue.
  */
-apr_status_t h2_mplx_c1_client_rst(h2_mplx *m, int stream_id);
+apr_status_t h2_mplx_c1_client_rst(h2_mplx *m, int stream_id,
+                                   struct h2_stream *stream);
 
 /**
  * Get readonly access to a stream for a secondary connection.

diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index 066c73ad98bc6f704f11452e90958b4eacdeae58..b6f6e7c01fb53d6733f15fb9c9d236b7a313f6f1 100644 (file)
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -402,6 +402,10 @@ static int on_frame_recv_cb(nghttp2_session *ng2s,
                           H2_SSSN_STRM_MSG(session, frame->hd.stream_id,
                           "RST_STREAM by client, error=%d"),
                           (int)frame->rst_stream.error_code);
+            if (stream) {
+                rv = h2_stream_recv_frame(stream, NGHTTP2_RST_STREAM, frame->hd.flags,
+                    frame->hd.length + H2_FRAME_HDR_LEN);
+            }
             if (stream && stream->initiated_on) {
                 /* A stream reset on a request we sent it. Normal, when the
                  * client does not want it. */
@@ -410,7 +414,8 @@ static int on_frame_recv_cb(nghttp2_session *ng2s,
             else {
                 /* A stream reset on a request it sent us. Could happen in a browser
                  * when the user navigates away or cancels loading - maybe. */
-                h2_mplx_c1_client_rst(session->mplx, frame->hd.stream_id);
+                h2_mplx_c1_client_rst(session->mplx, frame->hd.stream_id,
+                                      stream);
             }
             ++session->streams_reset;
             break;
@@ -812,6 +817,17 @@ static apr_status_t session_cleanup(h2_session *session, const char *trigger)
                       "goodbye, clients will be confused, should not happen"));
     }
 
+    if (!h2_iq_empty(session->ready_to_process)) {
+        int sid;
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c,
+                      H2_SSSN_LOG(APLOGNO(), session,
+                      "cleanup, resetting %d streams in ready-to-process"),
+                      h2_iq_count(session->ready_to_process));
+        while ((sid = h2_iq_shift(session->ready_to_process)) > 0) {
+          h2_mplx_c1_client_rst(session->mplx, sid, get_stream(session, sid));
+        }
+    }
+
     transit(session, trigger, H2_SESSION_ST_CLEANUP);
     h2_mplx_c1_destroy(session->mplx);
     session->mplx = NULL;

diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c
index c419e2d85914288dfec46f455edae6425c8d7223..f6c92024519fea8abe0ca0d03a04869c26a81d53 100644 (file)
--- a/modules/http2/h2_stream.c
+++ b/modules/http2/h2_stream.c
@@ -125,7 +125,7 @@ static int trans_on_event[][H2_SS_MAX] = {
 { S_XXX, S_ERR,  S_ERR,  S_CL_L, S_CLS,  S_XXX,  S_XXX,  S_XXX, },/* EV_CLOSED_L*/
 { S_ERR, S_ERR,  S_ERR,  S_CL_R, S_ERR,  S_CLS,  S_NOP,  S_NOP, },/* EV_CLOSED_R*/
 { S_CLS, S_CLS,  S_CLS,  S_CLS,  S_CLS,  S_CLS,  S_NOP,  S_NOP, },/* EV_CANCELLED*/
-{ S_NOP, S_XXX,  S_XXX,  S_XXX,  S_XXX,  S_CLS,  S_CLN,  S_XXX, },/* EV_EOS_SENT*/
+{ S_NOP, S_XXX,  S_XXX,  S_XXX,  S_XXX,  S_CLS,  S_CLN,  S_NOP, },/* EV_EOS_SENT*/
 { S_NOP, S_XXX,  S_CLS,  S_XXX,  S_XXX,  S_CLS,  S_XXX,  S_XXX, },/* EV_IN_ERROR*/
 };