http/2: unstick uploads

- refs #11157 and #11175 where uploads get stuck or lead to RST streams
- fixes our h2 send behaviour to continue sending in the nghttp2 session
  as long as it wants to. This will empty our send buffer as long as
  the remote stream/connection window allows.
- in case the window is exhausted, the data remaining in the send buffer
  will wait for a WINDOW_UPDATE from the server. Which is a socket event
  that engages our transfer loop again
- the problem in the issue was that we did not exhaust the window, but
  left data in the sendbuffer and no further socket events did happen.
  The server was just waiting for us to send more.
- relatedly, there was an issue fixed that closing a stream with KEEP_HOLD
  set kept the transfer from shutting down - as it should have - leading
  to a timeout.

Closes #11176

diff --git a/lib/http2.c b/lib/http2.c
index 2dc032f233c3f89e709b28b3cce2f17476123e70..c666192fc8af12bad92caca045582eb10e38104f 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -1200,6 +1200,7 @@ static int on_stream_close(nghttp2_session *session, int32_t stream_id,
   stream->error = error_code;
   if(stream->error)
     stream->reset = TRUE;
+  data_s->req.keepon &= ~KEEP_SEND_HOLD;
 
   drain_stream(cf, data_s, stream);
 
@@ -1676,7 +1677,9 @@ static CURLcode h2_progress_egress(struct Curl_cfilter *cf,
       goto out;
   }
 
-  rv = nghttp2_session_send(ctx->h2);
+  while(!rv && nghttp2_session_want_write(ctx->h2))
+    rv = nghttp2_session_send(ctx->h2);
+
 out:
   if(nghttp2_is_fatal(rv)) {
     DEBUGF(LOG_CF(data, cf, "nghttp2_session_send error (%s)%d",
@@ -1996,13 +1999,6 @@ static ssize_t cf_h2_send(struct Curl_cfilter *cf, struct Curl_easy *data,
   CF_DATA_SAVE(save, cf, data);
 
   if(stream && stream->id != -1) {
-    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] cf_send: win %u/%u",
-                stream->id,
-                nghttp2_session_get_remote_window_size(ctx->h2),
-                nghttp2_session_get_stream_remote_window_size(
-                  ctx->h2, stream->id)
-           ));
-
     if(stream->close_handled) {
       infof(data, "stream %u closed", stream->id);
       *err = CURLE_HTTP2_STREAM;
@@ -2021,6 +2017,8 @@ static ssize_t cf_h2_send(struct Curl_cfilter *cf, struct Curl_easy *data,
         goto out;
       nwritten = 0;
     }
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%u] bufq_write(len=%zu) -> %zd, %d",
+                  stream->id, len, nwritten, *err));
 
     if(!Curl_bufq_is_empty(&stream->sendbuf)) {
       rv = nghttp2_session_resume_data(ctx->h2, stream->id);
@@ -2063,14 +2061,16 @@ static ssize_t cf_h2_send(struct Curl_cfilter *cf, struct Curl_easy *data,
       DEBUGF(LOG_CF(data, cf, "[h2sid=%d] cf_send: win %u/%zu",
              stream->id,
              nghttp2_session_get_remote_window_size(ctx->h2), rwin));
-        if(rwin == 0) {
-          /* We cannot upload more as the stream's remote window size
-           * is 0. We need to receive WIN_UPDATEs before we can continue.
-           */
-          data->req.keepon |= KEEP_SEND_HOLD;
-          DEBUGF(LOG_CF(data, cf, "[h2sid=%d] holding send as remote flow "
-                 "window is exhausted", stream->id));
-        }
+      if(rwin == 0) {
+        /* We cannot upload more as the stream's remote window size
+         * is 0. We need to receive WIN_UPDATEs before we can continue.
+         */
+        data->req.keepon |= KEEP_SEND_HOLD;
+        DEBUGF(LOG_CF(data, cf, "[h2sid=%d] holding send as remote flow "
+               "window is exhausted", stream->id));
+      }
+      nwritten = -1;
+      *err = CURLE_AGAIN;
     }
     /* handled writing BODY for open stream. */
     goto out;
@@ -2109,8 +2109,24 @@ static ssize_t cf_h2_send(struct Curl_cfilter *cf, struct Curl_easy *data,
   }
 
 out:
-  DEBUGF(LOG_CF(data, cf, "[h2sid=%d] cf_send -> %zd, %d",
-         stream? stream->id : -1, nwritten, *err));
+  if(stream) {
+    DEBUGF(LOG_CF(data, cf, "[h2sid=%d] cf_send(len=%zu) -> %zd, %d, "
+                  "buffered=%zu, upload_left=%zu, stream-window=%d, "
+                  "connection-window=%d",
+                  stream->id, len, nwritten, *err,
+                  Curl_bufq_len(&stream->sendbuf),
+                  (ssize_t)stream->upload_left,
+                  nghttp2_session_get_stream_remote_window_size(
+                    ctx->h2, stream->id),
+                  nghttp2_session_get_remote_window_size(ctx->h2)));
+    drain_stream(cf, data, stream);
+  }
+  else {
+    DEBUGF(LOG_CF(data, cf, "cf_send(len=%zu) -> %zd, %d, "
+                  "connection-window=%d",
+                  len, nwritten, *err,
+                  nghttp2_session_get_remote_window_size(ctx->h2)));
+  }
   CF_DATA_RESTORE(cf, save);
   return nwritten;
 }
@@ -2313,6 +2329,7 @@ static bool cf_h2_data_pending(struct Curl_cfilter *cf,
   struct stream_ctx *stream = H2_STREAM_CTX(data);
 
   if(ctx && (!Curl_bufq_is_empty(&ctx->inbufq)
+            || (stream && !Curl_bufq_is_empty(&stream->sendbuf))
             || (stream && !Curl_bufq_is_empty(&stream->recvbuf))))
     return TRUE;
   return cf->next? cf->next->cft->has_data_pending(cf->next, data) : FALSE;

diff --git a/tests/http/test_07_upload.py b/tests/http/test_07_upload.py
index 37fa45cb9e404a789f47bc92a4dd6937e93c5fec..f28c644f01dbfc6e67d19f73356a4ef0438b2f8d 100644 (file)
--- a/tests/http/test_07_upload.py
+++ b/tests/http/test_07_upload.py
@@ -275,14 +275,14 @@ class TestUpload:
             '--resolve', f'{env.authority_for(env.domain1, proto)}:127.0.0.1',
             '--cacert', env.ca.cert_file,
             '--request', 'PUT',
-            '--max-time', '5', '-v',
+            '--max-time', '10', '-v',
             '--url', url,
             '--form', 'idList=12345678',
             '--form', 'pos=top',
             '--form', 'name=mr_test',
             '--form', f'fileSource=@{fdata};type=application/pdf',
         ])
-        assert r.exit_code == 0, f'{r}'
+        assert r.exit_code == 0, r.dump_logs()
         r.check_stats(1, 200)
 
     def check_download(self, count, srcfile, curl):