From: Stefan Eissing <icing@apache.org>
Date: Mon, 26 Oct 2015 15:10:22 +0000 (+0000)
Subject: improvements in stream reset handling and early stream close
X-Git-Tag: 2.5.0-alpha~2686
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7256caab695391e416c2c6272594b354f0595221;p=thirdparty%2Fapache%2Fhttpd.git

improvements in stream reset handling and early stream close

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

diff --git a/modules/http2/h2_io.c b/modules/http2/h2_io.c
index 4e61c0882be..9825a18d5f2 100644
--- a/modules/http2/h2_io.c
+++ b/modules/http2/h2_io.c
@@ -91,6 +91,10 @@ apr_status_t h2_io_in_read(h2_io *io, apr_bucket_brigade *bb,
     apr_bucket *last;
     apr_status_t status;
 
+    if (io->rst_error) {
+        return APR_ECONNABORTED;
+    }
+    
     if (!io->bbin || APR_BRIGADE_EMPTY(io->bbin)) {
         return io->eos_in? APR_EOF : APR_EAGAIN;
     }
@@ -113,6 +117,10 @@ apr_status_t h2_io_in_read(h2_io *io, apr_bucket_brigade *bb,
 
 apr_status_t h2_io_in_write(h2_io *io, apr_bucket_brigade *bb)
 {
+    if (io->rst_error) {
+        return APR_ECONNABORTED;
+    }
+    
     if (io->eos_in) {
         return APR_EOF;
     }
@@ -129,6 +137,10 @@ apr_status_t h2_io_in_write(h2_io *io, apr_bucket_brigade *bb)
 
 apr_status_t h2_io_in_close(h2_io *io)
 {
+    if (io->rst_error) {
+        return APR_ECONNABORTED;
+    }
+    
     if (io->bbin) {
         APR_BRIGADE_INSERT_TAIL(io->bbin, 
                                 apr_bucket_eos_create(io->bbin->bucket_alloc));
@@ -143,6 +155,10 @@ apr_status_t h2_io_out_readx(h2_io *io,
 {
     apr_status_t status;
     
+    if (io->rst_error) {
+        return APR_ECONNABORTED;
+    }
+    
     if (io->eos_out) {
         *plen = 0;
         *peos = 1;
@@ -169,6 +185,10 @@ apr_status_t h2_io_out_write(h2_io *io, apr_bucket_brigade *bb,
     apr_status_t status;
     int start_allowed;
     
+    if (io->rst_error) {
+        return APR_ECONNABORTED;
+    }
+
     if (io->eos_out) {
         apr_off_t len;
         /* We have already delivered an EOS bucket to a reader, no
@@ -199,9 +219,6 @@ apr_status_t h2_io_out_write(h2_io *io, apr_bucket_brigade *bb,
      */
     start_allowed = *pfile_handles_allowed;
 
-    if (io->rst_error) {
-        return APR_ECONNABORTED;
-    }
     status = h2_util_move(io->bbout, bb, maxlen, pfile_handles_allowed, 
                           "h2_io_out_write");
     /* track # file buckets moved into our pool */
@@ -214,6 +231,9 @@ apr_status_t h2_io_out_write(h2_io *io, apr_bucket_brigade *bb,
 
 apr_status_t h2_io_out_close(h2_io *io)
 {
+    if (io->rst_error) {
+        return APR_ECONNABORTED;
+    }
     if (!io->eos_out && !h2_util_has_eos(io->bbout, 0)) {
         APR_BRIGADE_INSERT_TAIL(io->bbout, 
                                 apr_bucket_eos_create(io->bbout->bucket_alloc));
diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c
index 68d307a0fdd..b9ddfb00724 100644
--- a/modules/http2/h2_mplx.c
+++ b/modules/http2/h2_mplx.c
@@ -29,6 +29,7 @@
 #include "h2_private.h"
 #include "h2_config.h"
 #include "h2_conn.h"
+#include "h2_h2.h"
 #include "h2_io.h"
 #include "h2_io_set.h"
 #include "h2_response.h"
@@ -288,18 +289,21 @@ apr_status_t h2_mplx_cleanup_stream(h2_mplx *m, h2_stream *stream)
     status = apr_thread_mutex_lock(m->lock);
     if (APR_SUCCESS == status) {
         h2_io *io = h2_io_set_get(m->stream_ios, stream->id);
-        if (!io || io->task_done) {
-            /* No more io or task already done -> cleanup immediately */
-            stream_destroy(m, stream, io);
-        }
-        else {
+        if (io) {
+            /* Remove io from ready set, we will never submit it */
+            h2_io_set_remove(m->ready_ios, io);
             if (stream->rst_error) {
                 /* Forward error code to fail any further attempt to
                  * write to io */
                 h2_io_rst(io, stream->rst_error);
             }
-            /* Remove io from ready set (if there), since we will never submit it */
-            h2_io_set_remove(m->ready_ios, io);
+        }
+        
+        if (!io || io->task_done) {
+            /* No more io or task already done -> cleanup immediately */
+            stream_destroy(m, stream, io);
+        }
+        else {
             /* Add stream to closed set for cleanup when task is done */
             h2_stream_set_add(m->closed, stream);
         }
@@ -515,18 +519,21 @@ h2_stream *h2_mplx_next_submit(h2_mplx *m, h2_stream_set *streams)
                     h2_stream_set_response(stream, io->response, io->bbout);
                 }
                 
-                if (io->output_drained) {
-                    apr_thread_cond_signal(io->output_drained);
-                }
             }
             else {
-                ap_log_cerror(APLOG_MARK, APLOG_WARNING, 0, m->c, APLOGNO(02953) 
-                              "h2_mplx(%ld): stream for response %d not found",
-                              m->id, io->id);
                 /* We have the io ready, but the stream has gone away, maybe
                  * reset by the client. Should no longer happen since such
                  * streams should clear io's from the ready queue.
                  */
+                ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, m->c, APLOGNO(02953) 
+                              "h2_mplx(%ld): stream for response %d closed, "
+                              "resetting io to close request processing",
+                              m->id, io->id);
+                h2_io_rst(io, H2_ERR_STREAM_CLOSED);
+            }
+            
+            if (io->output_drained) {
+                apr_thread_cond_signal(io->output_drained);
             }
         }
         apr_thread_mutex_unlock(m->lock);
diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index d8a8735afac..718fb218677 100644
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -168,7 +168,7 @@ static int on_data_chunk_recv_cb(nghttp2_session *ngh2, uint8_t flags,
                   session->id, stream_id, (int)len);
     if (status != APR_SUCCESS) {
         rv = nghttp2_submit_rst_stream(ngh2, NGHTTP2_FLAG_NONE, stream_id,
-                                       NGHTTP2_INTERNAL_ERROR);
+                                       H2_STREAM_RST(stream, H2_ERR_INTERNAL_ERROR));
         if (nghttp2_is_fatal(rv)) {
             return NGHTTP2_ERR_CALLBACK_FAILURE;
         }
@@ -931,17 +931,18 @@ apr_status_t h2_session_write(h2_session *session, apr_interval_time_t timeout)
     }
     
     /* If we have responses ready, submit them now. */
-    while ((stream = h2_mplx_next_submit(session->mplx, 
-                                         session->streams)) != NULL) {
+    while (!session->aborted 
+           && (stream = h2_mplx_next_submit(session->mplx, session->streams)) != NULL) {
         status = h2_session_handle_response(session, stream);
         flush_output = 1;
     }
     
-    if (h2_session_resume_streams_with_data(session) > 0) {
+    if (!session->aborted && h2_session_resume_streams_with_data(session) > 0) {
         flush_output = 1;
     }
     
-    if (!flush_output && timeout > 0 && !h2_session_want_write(session)) {
+    if (!session->aborted && !flush_output 
+        && timeout > 0 && !h2_session_want_write(session)) {
         status = h2_mplx_out_trywait(session->mplx, timeout, session->iowait);
 
         if (status != APR_TIMEUP
@@ -1151,13 +1152,10 @@ apr_status_t h2_session_handle_response(h2_session *session, h2_stream *stream)
     if (stream->response && stream->response->ngheader) {
         rv = submit_response(session, stream->response);
     }
-    else if (stream->rst_error) {
-        rv = nghttp2_submit_rst_stream(session->ngh2, NGHTTP2_FLAG_NONE,
-                                       stream->id, stream->rst_error);
-    }
     else {
         rv = nghttp2_submit_rst_stream(session->ngh2, NGHTTP2_FLAG_NONE,
-                                       stream->id, NGHTTP2_PROTOCOL_ERROR);
+                                       stream->id, 
+                                       H2_STREAM_RST(stream, H2_ERR_PROTOCOL_ERROR));
     }
     
     if (nghttp2_is_fatal(rv)) {
@@ -1179,27 +1177,6 @@ int h2_session_is_done(h2_session *session)
                 && !nghttp2_session_want_write(session->ngh2)));
 }
 
-static int log_stream(void *ctx, h2_stream *stream)
-{
-    h2_session *session = (h2_session *)ctx;
-    AP_DEBUG_ASSERT(session);
-    ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
-                  "h2_stream(%ld-%d): in set, suspended=%d, aborted=%d, "
-                  "has_data=%d",
-                  session->id, stream->id, stream->suspended, stream->aborted,
-                  h2_mplx_out_has_data_for(session->mplx, stream->id));
-    return 1;
-}
-
-void h2_session_log_stats(h2_session *session)
-{
-    AP_DEBUG_ASSERT(session);
-    ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c,
-                  "h2_session(%ld): %d open streams",
-                  session->id, (int)h2_stream_set_size(session->streams));
-    h2_stream_set_iter(session->streams, log_stream, session);
-}
-
 static int frame_print(const nghttp2_frame *frame, char *buffer, size_t maxlen)
 {
     char scratch[128];
diff --git a/modules/http2/h2_session.h b/modules/http2/h2_session.h
index 12768a90fb3..ddf29a06df1 100644
--- a/modules/http2/h2_session.h
+++ b/modules/http2/h2_session.h
@@ -158,6 +158,4 @@ apr_status_t h2_session_handle_response(h2_session *session,
 /* Get the h2_stream for the given stream idenrtifier. */
 struct h2_stream *h2_session_get_stream(h2_session *session, int stream_id);
 
-void h2_session_log_stats(h2_session *session);
-
 #endif /* defined(__mod_h2__h2_session__) */
diff --git a/modules/http2/h2_stream.h b/modules/http2/h2_stream.h
index c2bb9af7494..f885bc439f9 100644
--- a/modules/http2/h2_stream.h
+++ b/modules/http2/h2_stream.h
@@ -70,6 +70,8 @@ struct h2_stream {
 };
 
 
+#define H2_STREAM_RST(s, def)    (s->rst_error? s->rst_error : (def))
+
 h2_stream *h2_stream_create(int id, apr_pool_t *pool, struct h2_mplx *m);
 
 apr_status_t h2_stream_destroy(h2_stream *stream);
diff --git a/modules/http2/h2_version.h b/modules/http2/h2_version.h
index dc7c5772211..10219daf191 100644
--- a/modules/http2/h2_version.h
+++ b/modules/http2/h2_version.h
@@ -20,7 +20,7 @@
  * @macro
  * Version number of the h2 module as c string
  */
-#define MOD_HTTP2_VERSION "1.0.1-DEV"
+#define MOD_HTTP2_VERSION "1.0.2-DEV"
 
 /**
  * @macro
@@ -28,7 +28,7 @@
  * 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 0x010001
+#define MOD_HTTP2_VERSION_NUM 0x010002
 
 
 #endif /* mod_h2_h2_version_h */