merged latest session shutdown changes from trunk

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.17-protocols-http2@1704265 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 105 insertions, 76 deletions

diff --git a/modules/http2/h2_conn.c b/modules/http2/h2_conn.c
index 38e5c11a28..8bffbc4269 100644
--- a/modules/http2/h2_conn.c
+++ b/modules/http2/h2_conn.c
@@ -288,24 +288,24 @@ apr_status_t h2_session_process(h2_session *session)
                                   || session->frames_received <= 1)?
                                  APR_BLOCK_READ : APR_NONBLOCK_READ);
         switch (status) {
-            case APR_SUCCESS:
-                /* successful read, reset our idle timers */
+            case APR_SUCCESS:       /* successful read, reset our idle timers */
                 have_read = 1;
                 wait_micros = 0;
                 break;
-            case APR_EAGAIN:
+            case APR_EAGAIN:              /* non-blocking read, nothing there */
                 break;
-            case APR_EBADF:
+            case APR_EBADF:               /* connection is not there any more */
             case APR_EOF:
             case APR_ECONNABORTED:
             case APR_ECONNRESET:
+            case APR_TIMEUP:                       /* blocked read, timed out */
                 ap_log_cerror( APLOG_MARK, APLOG_DEBUG, status, session->c,
                               "h2_session(%ld): reading",
                               session->id);
                 h2_session_abort(session, status, 0);
                 break;
             default:
-                ap_log_cerror( APLOG_MARK, APLOG_WARNING, status, session->c,
+                ap_log_cerror( APLOG_MARK, APLOG_INFO, status, session->c,
                               APLOGNO(02950) 
                               "h2_session(%ld): error reading, terminating",
                               session->id);
diff --git a/modules/http2/h2_conn_io.c b/modules/http2/h2_conn_io.c
index 129456ebc3..08d00a4f3a 100644
--- a/modules/http2/h2_conn_io.c
+++ b/modules/http2/h2_conn_io.c
@@ -267,14 +267,7 @@ apr_status_t h2_conn_io_write(h2_conn_io *io,
     }
     else {
         status = apr_brigade_write(io->output, flush_out, io, buf, length);
-        if (status == APR_SUCCESS
-            || APR_STATUS_IS_ECONNABORTED(status)
-            || APR_STATUS_IS_EPIPE(status)) {
-            /* These are all fine and no reason for concern. Everything else
-             * is interesting. */
-            status = APR_SUCCESS;
-        }
-        else {
+        if (status != APR_SUCCESS) {
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, status, io->connection,
                           "h2_conn_io: write error");
         }
@@ -302,9 +295,7 @@ apr_status_t h2_conn_io_flush(h2_conn_io *io)
         /* Send it out through installed filters (TLS) to the client */
         status = flush_out(io->output, io);
         
-        if (status == APR_SUCCESS
-            || APR_STATUS_IS_ECONNABORTED(status)
-            || APR_STATUS_IS_EPIPE(status)) {
+        if (status == APR_SUCCESS) {
             /* These are all fine and no reason for concern. Everything else
              * is interesting. */
             io->unflushed = 0;
diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index 8c8bced32a..c3456a0654 100644
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -64,6 +64,9 @@ static int stream_open(h2_session *session, int stream_id)
     stream = h2_mplx_open_io(session->mplx, stream_id);
     if (stream) {
         h2_stream_set_add(session->streams, stream);
+        if (stream->id > session->max_stream_received) {
+            session->max_stream_received = stream->id;
+        }
         
         ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
                       "h2_session: stream(%ld-%d): opened",
@@ -223,10 +226,25 @@ static int on_frame_not_send_cb(nghttp2_session *ngh2,
     return 0;
 }
 
-static apr_status_t stream_destroy(h2_session *session, h2_stream *stream) 
+static apr_status_t stream_destroy(h2_session *session, 
+                                   h2_stream *stream,
+                                   uint32_t error_code) 
 {
-    ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
-                  "h2_stream(%ld-%d): closing", session->id, (int)stream->id);
+    if (!error_code) {
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
+                      "h2_stream(%ld-%d): handled, closing", 
+                      session->id, (int)stream->id);
+        if (stream->id > session->max_stream_handled) {
+            session->max_stream_handled = stream->id;
+        }
+    }
+    else {
+        ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
+                      "h2_stream(%ld-%d): closing with err=%d %s", 
+                      session->id, (int)stream->id, (int)error_code,
+                      nghttp2_strerror(error_code));
+    }
+    
     h2_stream_set_remove(session->streams, stream);
     return h2_mplx_cleanup_stream(session->mplx, stream);
 }
@@ -243,7 +261,7 @@ static int on_stream_close_cb(nghttp2_session *ngh2, int32_t stream_id,
     }
     stream = h2_stream_set_get(session->streams, stream_id);
     if (stream) {
-        stream_destroy(session, stream);
+        stream_destroy(session, stream, error_code);
     }
     
     if (error_code) {
@@ -655,50 +673,43 @@ void h2_session_destroy(h2_session *session)
     }
 }
 
-apr_status_t h2_session_goaway(h2_session *session, apr_status_t reason)
-{
-    apr_status_t status = APR_SUCCESS;
-    int rv;
-    AP_DEBUG_ASSERT(session);
-    if (session->aborted) {
-        return APR_EINVAL;
-    }
-    
-    rv = 0;
-    if (reason == APR_SUCCESS) {
-        rv = nghttp2_submit_shutdown_notice(session->ngh2);
-    }
-    else {
-        int err = 0;
-        int last_id = nghttp2_session_get_last_proc_stream_id(session->ngh2);
-        rv = nghttp2_submit_goaway(session->ngh2, last_id,
-                                   NGHTTP2_FLAG_NONE, err, NULL, 0);
-    }
-    if (rv != 0) {
-        status = APR_EGENERAL;
-        ap_log_cerror(APLOG_MARK, APLOG_ERR, status, session->c,
-                    APLOGNO(02930) "session(%ld): submit goaway: %s",
-                      session->id, nghttp2_strerror(rv));
-    }
-    return status;
-}
-
 static apr_status_t h2_session_abort_int(h2_session *session, int reason)
 {
     AP_DEBUG_ASSERT(session);
     if (!session->aborted) {
         session->aborted = 1;
         if (session->ngh2) {
-            if (reason) {
-                ap_log_cerror(APLOG_MARK, (reason == NGHTTP2_ERR_EOF)?
-                              APLOG_DEBUG : APLOG_INFO, 0, session->c,
+            
+            if (!reason) {
+                nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE, 
+                                      session->max_stream_received, 
+                                      reason, NULL, 0);
+                nghttp2_session_send(session->ngh2);
+                h2_conn_io_flush(&session->io);
+            }
+            else {
+                const char *err = nghttp2_strerror(reason);
+                
+                ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c,
                               "session(%ld): aborting session, reason=%d %s",
-                              session->id, reason, nghttp2_strerror(reason));
+                              session->id, reason, err);
+                
+                if (NGHTTP2_ERR_EOF == reason) {
+                    /* This is our way of indication that the connection is
+                     * gone. No use to send any GOAWAY frames. */
+                    nghttp2_session_terminate_session(session->ngh2, reason);
+                }
+                else {
+                    /* The connection might still be there and we shut down
+                     * with GOAWAY and reason information. */
+                     nghttp2_submit_goaway(session->ngh2, NGHTTP2_FLAG_NONE, 
+                                           session->max_stream_received, 
+                                           reason, (const uint8_t *)err, 
+                                           strlen(err));
+                     nghttp2_session_send(session->ngh2);
+                     h2_conn_io_flush(&session->io);
+                }
             }
-            nghttp2_session_terminate_session(session->ngh2, reason);
-            nghttp2_submit_goaway(session->ngh2, 0, 0, reason, NULL, 0);
-            nghttp2_session_send(session->ngh2);
-            h2_conn_io_flush(&session->io);
         }
         h2_mplx_abort(session->mplx);
     }
@@ -714,10 +725,12 @@ apr_status_t h2_session_abort(h2_session *session, apr_status_t reason, int rv)
             case APR_ENOMEM:
                 rv = NGHTTP2_ERR_NOMEM;
                 break;
-            case APR_EOF:
-                rv = 0;
+            case APR_SUCCESS:                            /* all fine, just... */
+            case APR_EOF:                         /* client closed its end... */
+            case APR_TIMEUP:                          /* got bored waiting... */
+                rv = 0;                            /* ...gracefully shut down */
                 break;
-            case APR_EBADF:
+            case APR_EBADF:        /* connection unusable, terminate silently */
             case APR_ECONNABORTED:
                 rv = NGHTTP2_ERR_EOF;
                 break;
@@ -1006,7 +1019,7 @@ apr_status_t h2_session_read(h2_session *session, apr_read_type_e block)
 apr_status_t h2_session_close(h2_session *session)
 {
     AP_DEBUG_ASSERT(session);
-    return h2_conn_io_flush(&session->io);
+    return session->aborted? APR_SUCCESS : h2_conn_io_flush(&session->io);
 }
 
 /* The session wants to send more DATA for the given stream.
diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h
index 77dd440a55..12768a90fb 100644
--- a/modules/http2/h2_session.h
+++ b/modules/http2/h2_session.h
@@ -71,44 +71,70 @@ struct h2_session {
     
     struct h2_stream_set *streams;  /* streams handled by this session */
     
+    int max_stream_received;        /* highest stream id created */
+    int max_stream_handled;         /* highest stream id handled successfully */
+    
     struct nghttp2_session *ngh2;   /* the nghttp2 session (internal use) */
     struct h2_workers *workers;     /* for executing stream tasks */
 };
 
 
-/* Create a new h2_session for the given connection (mode 'h2').
+/**
+ * Create a new h2_session for the given connection.
  * The session will apply the configured parameter.
+ * @param c       the connection to work on
+ * @param cfg     the module config to apply
+ * @param workers the worker pool to use
+ * @return the created session
  */
 h2_session *h2_session_create(conn_rec *c, struct h2_config *cfg, 
                               struct h2_workers *workers);
 
-/* Create a new h2_session for the given request (mode 'h2c').
+/**
+ * Create a new h2_session for the given request.
  * The session will apply the configured parameter.
+ * @param r       the request that was upgraded
+ * @param cfg     the module config to apply
+ * @param workers the worker pool to use
+ * @return the created session
  */
 h2_session *h2_session_rcreate(request_rec *r, struct h2_config *cfg,
                                struct h2_workers *workers);
 
-/* Destroy the session and all object it still contains. This will not
- * destroy h2_task instances that not finished yet. */
+/**
+ * Destroy the session and all objects it still contains. This will not
+ * destroy h2_task instances that have not finished yet. 
+ * @param session the session to destroy
+ */
 void h2_session_destroy(h2_session *session);
 
-/* Called once at start of session. Performs initial client thingies. */
+/**
+ * Called once at start of session. 
+ * Sets up the session and sends the initial SETTINGS frame.
+ * @param session the session to start
+ * @param rv error codes in libnghttp2 lingo are returned here
+ * @return APR_SUCCESS if all went well
+ */
 apr_status_t h2_session_start(h2_session *session, int *rv);
 
-/* Return != 0 iff session is finished and connection can be closed.
+/**
+ * Determine if session is finished.
+ * @return != 0 iff session is finished and connection can be closed.
  */
 int h2_session_is_done(h2_session *session);
 
-/* Called when the session will shutdown after all open streams
- * are handled. New streams will no longer be accepted. 
- * Call with reason APR_SUCCESS to initiate a graceful shutdown. */
-apr_status_t h2_session_goaway(h2_session *session, apr_status_t reason);
-
-/* Called when an error occured and the session needs to shut down.
- * Status indicates the reason of the error. */
+/**
+ * Called when an error occured and the session needs to shut down.
+ * @param session the session to shut down
+ * @param reason  the apache status that caused the shutdown
+ * @param rv      the nghttp2 reason for shutdown, set to 0 if you have none.
+ *
+ */
 apr_status_t h2_session_abort(h2_session *session, apr_status_t reason, int rv);
 
-/* Called before a session gets destroyed, might flush output etc. */
+/**
+ * Called before a session gets destroyed, might flush output etc. 
+ */
 apr_status_t h2_session_close(h2_session *session);
 
 /* Read more data from the client connection. Used normally with blocking
diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c
index 08b8f5a3f8..52781d8474 100644
--- a/modules/http2/h2_stream.c
+++ b/modules/http2/h2_stream.c
@@ -61,7 +61,7 @@ h2_stream *h2_stream_create(int id, apr_pool_t *pool, struct h2_mplx *m)
     return stream;
 }
 
-void h2_stream_cleanup(h2_stream *stream)
+static void h2_stream_cleanup(h2_stream *stream)
 {
     if (stream->request) {
         h2_request_destroy(stream->request);
diff --git a/modules/http2/h2_stream.h b/modules/http2/h2_stream.h
index f6bd71a5f8..0608f2f340 100644
--- a/modules/http2/h2_stream.h
+++ b/modules/http2/h2_stream.h
@@ -72,7 +72,6 @@ struct h2_stream {
 h2_stream *h2_stream_create(int id, apr_pool_t *pool, struct h2_mplx *m);
 
 apr_status_t h2_stream_destroy(h2_stream *stream);
-void h2_stream_cleanup(h2_stream *stream);
 
 apr_pool_t *h2_stream_detach_pool(h2_stream *stream);
 void h2_stream_attach_pool(h2_stream *stream, apr_pool_t *pool);