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 (nghttp2/nghttp2#1444) to work properly, to end up in the next release after 1.40.0. Fixes #4939 Closes #4940
curl · Feb 27, 2020 · 5fcb3ea · 5fcb3ea
1 parent 0922f76
commit 5fcb3ea
Show file tree

Hide file tree

Showing 5 changed files with 101 additions and 39 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -48,7 +48,7 @@ Host: %HOSTIP:%HTTPPORT
 Accept: */*
 Connection: Upgrade, HTTP2-Settings
 Upgrade: %H2CVER
-HTTP2-Settings: AAMAAABkAARAAAAAAAIAAAAA
+HTTP2-Settings: AAMAAABkAAQCAAAAAAIAAAAA
 
 </protocol>
 </verify>