Merge pull request #267 from icing/streams_send_timeout

Improve timeout handling of streams sending data
icing · Sep 18, 2023 · cc17584 · cc17584
2 parents 1815422 + 0c803be
commit cc17584
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 1 deletion.
diff --git a/mod_http2/h2_mplx.c b/mod_http2/h2_mplx.c
@@ -394,6 +394,31 @@ apr_status_t h2_mplx_c1_streams_do(h2_mplx *m, h2_mplx_stream_cb *cb, void *ctx)
     return APR_SUCCESS;
 }
 
+typedef struct {
+    int stream_count;
+    int stream_want_send;
+} stream_iter_aws_t;
+
+static int m_stream_want_send_data(void *ctx, void *stream)
+{
+    stream_iter_aws_t *x = ctx;
+    ++x->stream_count;
+    if (h2_stream_wants_send_data(stream))
+      ++x->stream_want_send;
+    return 1;
+}
+
+int h2_mplx_c1_all_streams_want_send_data(h2_mplx *m)
+{
+    stream_iter_aws_t x;
+    x.stream_count = 0;
+    x.stream_want_send = 0;
+    H2_MPLX_ENTER(m);
+    h2_ihash_iter(m->streams, m_stream_want_send_data, &x);
+    H2_MPLX_LEAVE(m);
+    return x.stream_count && (x.stream_count == x.stream_want_send);
+}
+
 static int m_report_stream_iter(void *ctx, void *val) {
     h2_mplx *m = ctx;
     h2_stream *stream = val;

diff --git a/mod_http2/h2_mplx.h b/mod_http2/h2_mplx.h
@@ -191,6 +191,11 @@ typedef int h2_mplx_stream_cb(struct h2_stream *s, void *userdata);
  */
 apr_status_t h2_mplx_c1_streams_do(h2_mplx *m, h2_mplx_stream_cb *cb, void *ctx);
 
+/**
+ * Return != 0 iff all open streams want to send data
+ */
+int h2_mplx_c1_all_streams_want_send_data(h2_mplx *m);
+
 /**
  * A stream has been RST_STREAM by the client. Abort
  * any processing going on and remove from processing

diff --git a/mod_http2/h2_session.c b/mod_http2/h2_session.c
@@ -1921,7 +1921,15 @@ apr_status_t h2_session_process(h2_session *session, int async)
             status = h2_mplx_c1_poll(session->mplx, session->s->timeout,
                                      on_stream_input, on_stream_output, session);
             if (APR_STATUS_IS_TIMEUP(status)) {
-                if (session->open_streams == 0) {
+                /* If we timeout without streams open, no new request from client
+                 * arrived.
+                 * If we timeout without nghttp2 wanting to write something, but
+                 * all open streams have something to send, it means we are
+                 * blocked on HTTP/2 flow control and the client did not send
+                 * WINDOW_UPDATEs to us. */
+                if (session->open_streams == 0 ||
+                    (!h2_session_want_send(session) &&
+                     h2_mplx_c1_all_streams_want_send_data(session->mplx))) {
                     h2_session_dispatch_event(session, H2_SESSION_EV_CONN_TIMEOUT, status, NULL);
                     break;
                 }

diff --git a/mod_http2/h2_stream.c b/mod_http2/h2_stream.c
@@ -1264,6 +1264,14 @@ int h2_stream_is_ready(h2_stream *stream)
     return 0;
 }
 
+int h2_stream_wants_send_data(h2_stream *stream)
+{
+    H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);
+    return h2_stream_is_ready(stream) &&
+           ((stream->out_buffer && !APR_BRIGADE_EMPTY(stream->out_buffer)) ||
+            (stream->output && !h2_beam_empty(stream->output)));
+}
+
 int h2_stream_is_at(const h2_stream *stream, h2_stream_state_t state)
 {
     H2_STRM_ASSERT_MAGIC(stream, H2_STRM_MAGIC_OK);

diff --git a/mod_http2/h2_stream.h b/mod_http2/h2_stream.h
@@ -332,6 +332,8 @@ const char *h2_stream_state_str(const h2_stream *stream);
  */
 int h2_stream_is_ready(h2_stream *stream);
 
+int h2_stream_wants_send_data(h2_stream *stream);
+
 #define H2_STRM_MSG(s, msg)     \
     "h2_stream(%d-%lu-%d,%s): "msg, s->session->child_num, \
     (unsigned long)s->session->id, s->id, h2_stream_state_str(s)