From: Stefan Eissing <icing@apache.org>
Date: Tue, 5 Jan 2016 15:23:17 +0000 (+0000)
Subject: always sending GOAWAY frame on session shutdown if not already done, GOAWAY frame... 
X-Git-Tag: 2.5.0-alpha~2458
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=7cfa8bf8d8dea5dc290e07ca422abe86dae33a2c;p=apache

always sending GOAWAY frame on session shutdown if not already done, GOAWAY frame with correct stream id, highest that started processing

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

diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c
index f016a5aa80..743f75455b 100644
--- a/modules/http2/h2_mplx.c
+++ b/modules/http2/h2_mplx.c
@@ -174,6 +174,17 @@ h2_mplx *h2_mplx_create(conn_rec *c, apr_pool_t *parent,
     return m;
 }
 
+int h2_mplx_get_max_stream_started(h2_mplx *m)
+{
+    int stream_id = 0;
+    
+    apr_thread_mutex_lock(m->lock);
+    stream_id = m->max_stream_started;
+    apr_thread_mutex_unlock(m->lock);
+    
+    return stream_id;
+}
+
 static void workers_register(h2_mplx *m)
 {
     /* Initially, there was ref count increase for this as well, but
@@ -341,11 +352,14 @@ static const h2_request *pop_request(h2_mplx *m)
 {
     const h2_request *req = NULL;
     int sid;
-    while (!req && (sid = h2_tq_shift(m->q)) > 0) {
+    while (!m->aborted && !req && (sid = h2_tq_shift(m->q)) > 0) {
         h2_io *io = h2_io_set_get(m->stream_ios, sid);
         if (io) {
             req = io->request;
             io->worker_started = 1;
+            if (sid > m->max_stream_started) {
+                m->max_stream_started = sid;
+            }
         }
     }
     return req;
diff --git a/modules/http2/h2_mplx.h b/modules/http2/h2_mplx.h
index 970b7265c3..f3edc51356 100644
--- a/modules/http2/h2_mplx.h
+++ b/modules/http2/h2_mplx.h
@@ -71,6 +71,8 @@ struct h2_mplx {
     struct h2_io_set *stream_ios;
     struct h2_io_set *ready_ios;
     
+    int max_stream_started;      /* highest stream id that started processing */
+
     apr_thread_mutex_t *lock;
     struct apr_thread_cond_t *added_output;
     struct apr_thread_cond_t *join_wait;
@@ -119,6 +121,14 @@ void h2_mplx_abort(h2_mplx *mplx);
 
 void h2_mplx_request_done(h2_mplx **pm, int stream_id, const struct h2_request **preq);
 
+/**
+ * Get the highest stream identifier that has been passed on to processing.
+ * Maybe 0 in case no stream has been processed yet.
+ * @param m the multiplexer
+ * @return highest stream identifier for which processing started
+ */
+int h2_mplx_get_max_stream_started(h2_mplx *m);
+
 /*******************************************************************************
  * IO lifetime of streams.
  ******************************************************************************/
diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index d12b110792..f0eb500aeb 100644
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -891,21 +891,21 @@ void h2_session_eoc_callback(h2_session *session)
 
 static apr_status_t h2_session_shutdown(h2_session *session, int reason)
 {
+    apr_status_t status = APR_SUCCESS;
+    
     AP_DEBUG_ASSERT(session);
     session->aborted = 1;
     if (session->state != H2_SESSION_ST_CLOSING
         && session->state != H2_SESSION_ST_ABORTED) {
-        if (session->client_goaway) {
-            /* client sent us a GOAWAY, just terminate */
-            nghttp2_session_terminate_session(session->ngh2, NGHTTP2_ERR_EOF);
-            ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
-                          "session(%ld): shutdown, GOAWAY from client", session->id);
+        h2_mplx_abort(session->mplx);
+        if (session->server_goaway) {
+            /* already sent one */
         }
         else if (!reason) {
             nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE, 
-                                  session->max_stream_received, 
+                                  h2_mplx_get_max_stream_started(session->mplx), 
                                   reason, NULL, 0);
-            nghttp2_session_send(session->ngh2);
+            status = nghttp2_session_send(session->ngh2);
             session->server_goaway = 1;
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
                           "session(%ld): shutdown, no err", session->id);
@@ -913,19 +913,18 @@ static apr_status_t h2_session_shutdown(h2_session *session, int reason)
         else {
             const char *err = nghttp2_strerror(reason);
             nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE, 
-                                  session->max_stream_received, 
+                                  h2_mplx_get_max_stream_started(session->mplx), 
                                   reason, (const uint8_t *)err, 
                                   strlen(err));
-            nghttp2_session_send(session->ngh2);
+            status = nghttp2_session_send(session->ngh2);
             session->server_goaway = 1;
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
                           "session(%ld): shutdown, err=%d '%s'",
                           session->id, reason, err);
         }
         session->state = H2_SESSION_ST_CLOSING;
-        h2_mplx_abort(session->mplx);
     }
-    return APR_SUCCESS;
+    return status;
 }
 
 void h2_session_abort(h2_session *session, apr_status_t status)
@@ -1099,24 +1098,20 @@ h2_stream *h2_session_get_stream(h2_session *session, int stream_id)
 
 void h2_session_close(h2_session *session)
 {
+    apr_status_t status = APR_SUCCESS;
     apr_bucket *b;
     conn_rec *c = session->c;
-    apr_status_t status;
     
     AP_DEBUG_ASSERT(session);
-    if (!session->aborted) {
-        h2_session_shutdown(session, 0);
+    if (!session->server_goaway) {
+        status = h2_session_shutdown(session, 0);
     }
     h2_session_cleanup(session);
 
-    ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, c,
+    ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, c,
                   "h2_session(%ld): writing eoc", c->id);
     b = h2_bucket_eoc_create(c->bucket_alloc, session);
-    status = h2_conn_io_write_eoc(&session->io, b);
-    if (status != APR_SUCCESS) {
-        ap_log_cerror(APLOG_MARK, APLOG_ERR, status, c,
-                      "h2_session(%ld): flushed eoc bucket", c->id);
-    } 
+    h2_conn_io_write_eoc(&session->io, b);
     /* and all is or will be destroyed */
 }
 
diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h
index b0652fa768..2dba10f720 100644
--- a/modules/http2/h2_session.h
+++ b/modules/http2/h2_session.h
@@ -91,7 +91,7 @@ typedef struct h2_session {
     int streams_reset;              /* number of http/2 streams reset by client */
     int streams_pushed;             /* number of http/2 streams pushed */
     int max_stream_received;        /* highest stream id created */
-    int max_stream_handled;         /* highest stream id handled successfully */
+    int max_stream_handled;         /* highest stream id completed */
     
     apr_size_t max_stream_count;    /* max number of open streams */
     apr_size_t max_stream_mem;      /* max buffer memory for a single stream */
diff --git a/modules/http2/h2_workers.c b/modules/http2/h2_workers.c
index 1b95cecbce..f7606dc4ed 100644
--- a/modules/http2/h2_workers.c
+++ b/modules/http2/h2_workers.c
@@ -322,7 +322,7 @@ apr_status_t h2_workers_register(h2_workers *workers, struct h2_mplx *m)
         ap_log_error(APLOG_MARK, APLOG_TRACE3, status, workers->s,
                      "h2_workers: register mplx(%ld)", m->id);
         if (in_list(workers, m)) {
-            ap_log_error(APLOG_MARK, APLOG_INFO, 0, workers->s,
+            ap_log_error(APLOG_MARK, APLOG_TRACE3, 0, workers->s,
                          "h2_workers: already registered mplx(%ld)", m->id);
             status = APR_EAGAIN;
         }