http2: make pausing/unpausing set/clear local stream window

This reduces the HTTP/2 window size to 32 MB since libcurl might have to
buffer up to this amount of data in memory and yet we don't want it set
lower to potentially impact tranfer performance on high speed networks.

Requires nghttp2 commit b3f85e2daa629
(https://github.com/nghttp2/nghttp2/pull/1444) to work properly, to end
up in the next release after 1.40.0.

Fixes #4939
Closes #4940

diff --git a/lib/easy.c b/lib/easy.c
index 56ba2b2bd9f19ea79f3b9df5f2bbd37c19134b54..454621076ab2bc015f53177c6b361d0f59f24c36 100644 (file)
--- a/lib/easy.c
+++ b/lib/easy.c
@@ -76,6 +76,7 @@
 #include "setopt.h"
 #include "http_digest.h"
 #include "system_win32.h"
+#include "http2.h"
 
 /* The last 3 #include files should be in this order */
 #include "curl_printf.h"
@@ -985,43 +986,47 @@ CURLcode curl_easy_pause(struct Curl_easy *data, int action)
   /* put it back in the keepon */
   k->keepon = newstate;
 
-  if(!(newstate & KEEP_RECV_PAUSE) && data->state.tempcount) {
-    /* there are buffers for sending that can be delivered as the receive
-       pausing is lifted! */
-    unsigned int i;
-    unsigned int count = data->state.tempcount;
-    struct tempbuf writebuf[3]; /* there can only be three */
-    struct connectdata *conn = data->conn;
-    struct Curl_easy *saved_data = NULL;
-
-    /* copy the structs to allow for immediate re-pausing */
-    for(i = 0; i < data->state.tempcount; i++) {
-      writebuf[i] = data->state.tempwrite[i];
-      data->state.tempwrite[i].buf = NULL;
-    }
-    data->state.tempcount = 0;
+  if(!(newstate & KEEP_RECV_PAUSE)) {
+    Curl_http2_stream_pause(data, FALSE);
+
+    if(data->state.tempcount) {
+      /* there are buffers for sending that can be delivered as the receive
+         pausing is lifted! */
+      unsigned int i;
+      unsigned int count = data->state.tempcount;
+      struct tempbuf writebuf[3]; /* there can only be three */
+      struct connectdata *conn = data->conn;
+      struct Curl_easy *saved_data = NULL;
+
+      /* copy the structs to allow for immediate re-pausing */
+      for(i = 0; i < data->state.tempcount; i++) {
+        writebuf[i] = data->state.tempwrite[i];
+        data->state.tempwrite[i].buf = NULL;
+      }
+      data->state.tempcount = 0;
 
-    /* set the connection's current owner */
-    if(conn->data != data) {
-      saved_data = conn->data;
-      conn->data = data;
-    }
+      /* set the connection's current owner */
+      if(conn->data != data) {
+        saved_data = conn->data;
+        conn->data = data;
+      }
 
-    for(i = 0; i < count; i++) {
-      /* even if one function returns error, this loops through and frees all
-         buffers */
-      if(!result)
-        result = Curl_client_write(conn, writebuf[i].type, writebuf[i].buf,
-                                   writebuf[i].len);
-      free(writebuf[i].buf);
-    }
+      for(i = 0; i < count; i++) {
+        /* even if one function returns error, this loops through and frees
+           all buffers */
+        if(!result)
+          result = Curl_client_write(conn, writebuf[i].type, writebuf[i].buf,
+                                     writebuf[i].len);
+        free(writebuf[i].buf);
+      }
 
-    /* recover previous owner of the connection */
-    if(saved_data)
-      conn->data = saved_data;
+      /* recover previous owner of the connection */
+      if(saved_data)
+        conn->data = saved_data;
 
-    if(result)
-      return result;
+      if(result)
+        return result;
+    }
   }
 
   /* if there's no error and we're not pausing both directions, we want

diff --git a/lib/http2.c b/lib/http2.c
index dffc7a2543d5e0f98c639187b2d14f56f287981f..72b38a3f66da62c7c0656f1573ec5f88689e0612 100644 (file)
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -55,7 +55,7 @@
 #define NGHTTP2_HAS_SET_LOCAL_WINDOW_SIZE 1
 #endif
 
-#define HTTP2_HUGE_WINDOW_SIZE (1 << 30)
+#define HTTP2_HUGE_WINDOW_SIZE (32 * 1024 * 1024) /* 32 MB */
 
 #ifdef DEBUG_HTTP2
 #define H2BUGF(x) x
@@ -1118,6 +1118,7 @@ static void populate_settings(struct connectdata *conn,
                               struct http_conn *httpc)
 {
   nghttp2_settings_entry *iv = httpc->local_settings;
+  DEBUGASSERT(conn->data);
 
   iv[0].settings_id = NGHTTP2_SETTINGS_MAX_CONCURRENT_STREAMS;
   iv[0].value = Curl_multi_max_concurrent_streams(conn->data->multi);
@@ -1554,8 +1555,12 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
     return ncopy;
   }
 
-  H2BUGF(infof(data, "http2_recv: easy %p (stream %u)\n",
-               data, stream->stream_id));
+  H2BUGF(infof(data, "http2_recv: easy %p (stream %u) win %u/%u\n",
+               data, stream->stream_id,
+               nghttp2_session_get_local_window_size(httpc->h2),
+               nghttp2_session_get_stream_local_window_size(httpc->h2,
+                                                            stream->stream_id)
+           ));
 
   if((data->state.drain) && stream->memlen) {
     H2BUGF(infof(data, "http2_recv: DRAIN %zu bytes stream %u!! (%p => %p)\n",
@@ -1586,7 +1591,6 @@ static ssize_t http2_recv(struct connectdata *conn, int sockindex,
     stream->pausedata += nread;
     stream->pauselen -= nread;
 
-    infof(data, "%zd data bytes written\n", nread);
     if(stream->pauselen == 0) {
       H2BUGF(infof(data, "Unpaused by stream %u\n", stream->stream_id));
       DEBUGASSERT(httpc->pause_stream_id == stream->stream_id);
@@ -2288,6 +2292,51 @@ CURLcode Curl_http2_switched(struct connectdata *conn,
   return CURLE_OK;
 }
 
+CURLcode Curl_http2_stream_pause(struct Curl_easy *data, bool pause)
+{
+  DEBUGASSERT(data);
+  DEBUGASSERT(data->conn);
+  /* if it isn't HTTP/2, we're done */
+  if(!data->conn->proto.httpc.h2)
+    return CURLE_OK;
+#ifdef NGHTTP2_HAS_SET_LOCAL_WINDOW_SIZE
+  else {
+    struct HTTP *stream = data->req.protop;
+    struct http_conn *httpc = &data->conn->proto.httpc;
+    uint32_t window = !pause * HTTP2_HUGE_WINDOW_SIZE;
+    int rv = nghttp2_session_set_local_window_size(httpc->h2,
+                                                   NGHTTP2_FLAG_NONE,
+                                                   stream->stream_id,
+                                                   window);
+    if(rv) {
+      failf(data, "nghttp2_session_set_local_window_size() failed: %s(%d)",
+            nghttp2_strerror(rv), rv);
+      return CURLE_HTTP2;
+    }
+
+    /* make sure the window update gets sent */
+    rv = h2_session_send(data, httpc->h2);
+    if(rv)
+      return CURLE_SEND_ERROR;
+
+    DEBUGF(infof(data, "Set HTTP/2 window size to %u for stream %u\n",
+                 window, stream->stream_id));
+
+#ifdef DEBUGBUILD
+    {
+      /* read out the stream local window again */
+      uint32_t window2 =
+        nghttp2_session_get_stream_local_window_size(httpc->h2,
+                                                     stream->stream_id);
+      DEBUGF(infof(data, "HTTP/2 window size is now %u for stream %u\n",
+                   window2, stream->stream_id));
+    }
+#endif
+  }
+#endif
+  return CURLE_OK;
+}
+
 CURLcode Curl_http2_add_child(struct Curl_easy *parent,
                               struct Curl_easy *child,
                               bool exclusive)

diff --git a/lib/http2.h b/lib/http2.h
index 12d36eef9b84684250bd9260173f79230b16cdaf..1989aff8267b6227ef25cbce534a0f58fcf5ce62 100644 (file)
--- a/lib/http2.h
+++ b/lib/http2.h
@@ -7,7 +7,7 @@
  *                            | (__| |_| |  _ <| |___
  *                             \___|\___/|_| \_\_____|
  *
- * Copyright (C) 1998 - 2019, Daniel Stenberg, <daniel@haxx.se>, et al.
+ * Copyright (C) 1998 - 2020, Daniel Stenberg, <daniel@haxx.se>, et al.
  *
  * This software is licensed as described in the file COPYING, which
  * you should have received as part of this distribution. The terms
@@ -58,6 +58,7 @@ CURLcode Curl_http2_add_child(struct Curl_easy *parent,
 void Curl_http2_remove_child(struct Curl_easy *parent,
                              struct Curl_easy *child);
 void Curl_http2_cleanup_dependencies(struct Curl_easy *data);
+CURLcode Curl_http2_stream_pause(struct Curl_easy *data, bool pause);
 
 /* returns true if the HTTP/2 stream error was HTTP_1_1_REQUIRED */
 bool Curl_h2_http_1_1_error(struct connectdata *conn);
@@ -74,6 +75,7 @@ bool Curl_h2_http_1_1_error(struct connectdata *conn);
 #define Curl_http2_add_child(x, y, z)
 #define Curl_http2_remove_child(x, y)
 #define Curl_http2_cleanup_dependencies(x)
+#define Curl_http2_stream_pause(x, y)
 #define Curl_h2_http_1_1_error(x) 0
 #endif
 

diff --git a/lib/sendf.c b/lib/sendf.c
index 51bbca75e3cf562ac14940aef75b938ea158821b..6ef47aa801d284de1ec7ea4656e35ea6ad9481ee 100644 (file)
--- a/lib/sendf.c
+++ b/lib/sendf.c
@@ -43,6 +43,7 @@
 #include "strerror.h"
 #include "select.h"
 #include "strdup.h"
+#include "http2.h"
 
 /* The last 3 #include files should be in this order */
 #include "curl_printf.h"
@@ -501,6 +502,9 @@ static CURLcode pausewrite(struct Curl_easy *data,
   unsigned int i;
   bool newtype = TRUE;
 
+  /* If this transfers over HTTP/2, pause the stream! */
+  Curl_http2_stream_pause(data, TRUE);
+
   if(s->tempcount) {
     for(i = 0; i< s->tempcount; i++) {
       if(s->tempwrite[i].type == type) {
@@ -529,6 +533,8 @@ static CURLcode pausewrite(struct Curl_easy *data,
     /* update the pointer and the size */
     s->tempwrite[i].buf = newptr;
     s->tempwrite[i].len = newlen;
+
+    len = newlen; /* for the debug output below */
   }
   else {
     dupl = Curl_memdup(ptr, len);

diff --git a/tests/data/test1800 b/tests/data/test1800
index 011018400132be7a7363d6d221f7b3e779b7193c..c308c99b0fd170b8e9cee91502be9ce0abdd8100 100644 (file)
--- a/tests/data/test1800
+++ b/tests/data/test1800
@@ -48,7 +48,7 @@ Host: %HOSTIP:%HTTPPORT
 Accept: */*\r
 Connection: Upgrade, HTTP2-Settings\r
 Upgrade: %H2CVER\r
-HTTP2-Settings: AAMAAABkAARAAAAAAAIAAAAA\r
+HTTP2-Settings: AAMAAABkAAQCAAAAAAIAAAAA\r
 \r
 </protocol>
 </verify>