aws_http_connection_update_window -> aws_http2_connection_update_window (#327)

TingDaoK · web-flow · commit 5d1871fb867a · 2021-09-27T09:26:49.000-07:00
- aws_http_connection_update_window -&gt; aws_http2_connection_update_window
- Error out when the wrong connection used as user cannot do anything to recover.
diff --git a/include/aws/http/connection.h b/include/aws/http/connection.h
@@ -296,6 +296,9 @@ struct aws_http_client_connection_options {
      * If a stream's flow-control window reaches 0, no further data will be received.
      * The user must call aws_http_stream_update_window() to increment the stream's
      * window and keep data flowing.
+     *
+     * If you created a HTTP/2 connection, it will also control the connection window
+     * management.
      */
     bool manual_window_management;
 
@@ -453,13 +456,6 @@ bool aws_http_connection_new_requests_allowed(const struct aws_http_connection *
 AWS_HTTP_API
 bool aws_http_connection_is_client(const struct aws_http_connection *connection);
 
-/**
- * DEPRECATED
- * TODO: Delete once this is removed from H2.
- */
-AWS_HTTP_API
-void aws_http_connection_update_window(struct aws_http_connection *connection, size_t increment_size);
-
 AWS_HTTP_API
 enum aws_http_version aws_http_connection_get_version(const struct aws_http_connection *connection);
 
@@ -537,7 +533,7 @@ int aws_http2_connection_ping(
  * @param out_settings fixed size array of aws_http2_setting gets set to the local settings
  */
 AWS_HTTP_API
-int aws_http2_connection_get_local_settings(
+void aws_http2_connection_get_local_settings(
     const struct aws_http_connection *http2_connection,
     struct aws_http2_setting out_settings[AWS_HTTP2_SETTINGS_COUNT]);
 
@@ -548,7 +544,7 @@ int aws_http2_connection_get_local_settings(
  * @param out_settings fixed size array of aws_http2_setting gets set to  the remote settings
  */
 AWS_HTTP_API
-int aws_http2_connection_get_remote_settings(
+void aws_http2_connection_get_remote_settings(
     const struct aws_http_connection *http2_connection,
     struct aws_http2_setting out_settings[AWS_HTTP2_SETTINGS_COUNT]);
 
@@ -562,6 +558,8 @@ int aws_http2_connection_get_remote_settings(
  * (http2_error=0, allow_more_streams=true), or to customize the final GOAWAY
  * frame that is sent by this connection.
  *
+ * The other end may not receive the goaway, if the connection already closed.
+ *
  * @param http2_connection HTTP/2 connection.
  * @param http2_error The HTTP/2 error code (RFC-7540 section 7) to send.
  *      `enum aws_http2_error_code` lists official codes.
@@ -610,6 +608,32 @@ int aws_http2_connection_get_received_goaway(
     uint32_t *out_http2_error,
     uint32_t *out_last_stream_id);
 
+/**
+ * Increment the connection's flow-control window to keep data flowing (HTTP/2 only).
+ *
+ * If the connection was created with `manual_window_management` set true,
+ * the flow-control window of the connection will shrink as body data is received for all the streams created on it.
+ * (headers, padding, and other metadata do not affect the window).
+ * The initial connection flow-control window is 65,535.
+ * Once the connection's flow-control window reaches to 0, all the streams on the connection stop receiving any further
+ * data.
+ *
+ * If `manual_window_management` is false, this call will have no effect.
+ * The connection maintains its flow-control windows such that
+ * no back-pressure is applied and data arrives as fast as possible.
+ *
+ * If you are not connected, this call will have no effect.
+ *
+ * Crashes when the connection is not http2 connection.
+ * The limit of the Maximum Size is 2**31 - 1. If the increment size cause the connection flow window exceeds the
+ * Maximum size, this call will result in the connection lost.
+ *
+ * @param http2_connection HTTP/2 connection.
+ * @param increment_size The size to increment for the connection's flow control window
+ */
+AWS_HTTP_API
+void aws_http2_connection_update_window(struct aws_http_connection *http2_connection, uint32_t increment_size);
+
 AWS_EXTERN_C_END
 
 #endif /* AWS_HTTP_CONNECTION_H */
diff --git a/include/aws/http/private/connection_impl.h b/include/aws/http/private/connection_impl.h
@@ -42,9 +42,9 @@ struct aws_http_connection_vtable {
     void (*close)(struct aws_http_connection *connection);
     bool (*is_open)(const struct aws_http_connection *connection);
     bool (*new_requests_allowed)(const struct aws_http_connection *connection);
-    void (*update_window)(struct aws_http_connection *connection, size_t increment_size);
 
     /* HTTP/2 specific functions */
+    void (*update_window)(struct aws_http_connection *connection, uint32_t increment_size);
     int (*change_settings)(
         struct aws_http_connection *http2_connection,
         const struct aws_http2_setting *settings_array,
diff --git a/include/aws/http/private/h2_connection.h b/include/aws/http/private/h2_connection.h
@@ -263,7 +263,7 @@ int aws_h2_connection_send_rst_and_close_reserved_stream(
     uint32_t h2_error_code);
 
 /**
- * Error happens while writing into channel, shutdown the connection.
+ * Error happens while writing into channel, shutdown the connection. Only called within the eventloop thread
  */
 void aws_h2_connection_shutdown_due_to_write_err(struct aws_h2_connection *connection, int error_code);
 
diff --git a/include/aws/http/private/h2_stream.h b/include/aws/http/private/h2_stream.h
@@ -80,9 +80,9 @@ struct aws_h2_stream {
         /* The window_update value for `thread_data.window_size_self` that haven't applied yet */
         size_t window_update_size;
 
-        /* The aws_http2_error_code user wanted to send to remote peer via rst_stream. */
-        uint32_t user_reset_error_code;
-
+        /* The combined aws_http2_error_code user wanted to send to remote peer via rst_stream and internal aws error
+         * code we want to inform user about. */
+        struct aws_h2err reset_error;
         bool reset_called;
 
         /* Simplified stream state. */
diff --git a/source/connection.c b/source/connection.c
@@ -269,23 +269,6 @@ bool aws_http_connection_is_server(const struct aws_http_connection *connection)
     return connection->server_data;
 }
 
-void aws_http_connection_update_window(struct aws_http_connection *connection, size_t increment_size) {
-    AWS_ASSERT(connection);
-    connection->vtable->update_window(connection, increment_size);
-}
-
-static int s_check_http2_connection(const struct aws_http_connection *http2_connection) {
-    if (http2_connection->http_version == AWS_HTTP_VERSION_2) {
-        return AWS_OP_SUCCESS;
-    } else {
-        AWS_LOGF_WARN(
-            AWS_LS_HTTP_CONNECTION,
-            "id=%p: HTTP/2 connection only function invoked on connection with other protocol, ignoring call.",
-            (void *)http2_connection);
-        return aws_raise_error(AWS_ERROR_INVALID_STATE);
-    }
-}
-
 int aws_http2_connection_change_settings(
     struct aws_http_connection *http2_connection,
     const struct aws_http2_setting *settings_array,
@@ -294,9 +277,7 @@ int aws_http2_connection_change_settings(
     void *user_data) {
     AWS_ASSERT(http2_connection);
     AWS_PRECONDITION(http2_connection->vtable);
-    if (s_check_http2_connection(http2_connection)) {
-        return AWS_OP_ERR;
-    }
+    AWS_FATAL_ASSERT(http2_connection->http_version == AWS_HTTP_VERSION_2);
     return http2_connection->vtable->change_settings(
         http2_connection, settings_array, num_settings, on_completed, user_data);
 }
@@ -308,9 +289,7 @@ int aws_http2_connection_ping(
     void *user_data) {
     AWS_ASSERT(http2_connection);
     AWS_PRECONDITION(http2_connection->vtable);
-    if (s_check_http2_connection(http2_connection)) {
-        return AWS_OP_ERR;
-    }
+    AWS_FATAL_ASSERT(http2_connection->http_version == AWS_HTTP_VERSION_2);
     return http2_connection->vtable->send_ping(http2_connection, optional_opaque_data, on_ack, user_data);
 }
 
@@ -321,9 +300,7 @@ int aws_http2_connection_send_goaway(
     const struct aws_byte_cursor *optional_debug_data) {
     AWS_ASSERT(http2_connection);
     AWS_PRECONDITION(http2_connection->vtable);
-    if (s_check_http2_connection(http2_connection)) {
-        return AWS_OP_ERR;
-    }
+    AWS_FATAL_ASSERT(http2_connection->http_version == AWS_HTTP_VERSION_2);
     return http2_connection->vtable->send_goaway(
         http2_connection, http2_error, allow_more_streams, optional_debug_data);
 }
@@ -336,9 +313,7 @@ int aws_http2_connection_get_sent_goaway(
     AWS_PRECONDITION(out_http2_error);
     AWS_PRECONDITION(out_last_stream_id);
     AWS_PRECONDITION(http2_connection->vtable);
-    if (s_check_http2_connection(http2_connection)) {
-        return AWS_OP_ERR;
-    }
+    AWS_FATAL_ASSERT(http2_connection->http_version == AWS_HTTP_VERSION_2);
     return http2_connection->vtable->get_sent_goaway(http2_connection, out_http2_error, out_last_stream_id);
 }
 
@@ -350,34 +325,33 @@ int aws_http2_connection_get_received_goaway(
     AWS_PRECONDITION(out_http2_error);
     AWS_PRECONDITION(out_last_stream_id);
     AWS_PRECONDITION(http2_connection->vtable);
-    if (s_check_http2_connection(http2_connection)) {
-        return AWS_OP_ERR;
-    }
+    AWS_FATAL_ASSERT(http2_connection->http_version == AWS_HTTP_VERSION_2);
     return http2_connection->vtable->get_received_goaway(http2_connection, out_http2_error, out_last_stream_id);
 }
 
-int aws_http2_connection_get_local_settings(
+void aws_http2_connection_get_local_settings(
     const struct aws_http_connection *http2_connection,
     struct aws_http2_setting out_settings[AWS_HTTP2_SETTINGS_COUNT]) {
     AWS_ASSERT(http2_connection);
     AWS_PRECONDITION(http2_connection->vtable);
-    if (s_check_http2_connection(http2_connection)) {
-        return AWS_OP_ERR;
-    }
+    AWS_FATAL_ASSERT(http2_connection->http_version == AWS_HTTP_VERSION_2);
     http2_connection->vtable->get_local_settings(http2_connection, out_settings);
-    return AWS_OP_SUCCESS;
 }
 
-int aws_http2_connection_get_remote_settings(
+void aws_http2_connection_get_remote_settings(
     const struct aws_http_connection *http2_connection,
     struct aws_http2_setting out_settings[AWS_HTTP2_SETTINGS_COUNT]) {
     AWS_ASSERT(http2_connection);
     AWS_PRECONDITION(http2_connection->vtable);
-    if (s_check_http2_connection(http2_connection)) {
-        return AWS_OP_ERR;
-    }
+    AWS_FATAL_ASSERT(http2_connection->http_version == AWS_HTTP_VERSION_2);
     http2_connection->vtable->get_remote_settings(http2_connection, out_settings);
-    return AWS_OP_SUCCESS;
+}
+
+void aws_http2_connection_update_window(struct aws_http_connection *http2_connection, uint32_t increment_size) {
+    AWS_ASSERT(http2_connection);
+    AWS_PRECONDITION(http2_connection->vtable);
+    AWS_FATAL_ASSERT(http2_connection->http_version == AWS_HTTP_VERSION_2);
+    http2_connection->vtable->update_window(http2_connection, increment_size);
 }
 
 struct aws_channel *aws_http_connection_get_channel(struct aws_http_connection *connection) {
diff --git a/source/h2_connection.c b/source/h2_connection.c
@@ -56,7 +56,7 @@ static struct aws_http_stream *s_connection_make_request(
 static void s_connection_close(struct aws_http_connection *connection_base);
 static bool s_connection_is_open(const struct aws_http_connection *connection_base);
 static bool s_connection_new_requests_allowed(const struct aws_http_connection *connection_base);
-static void s_connection_update_window(struct aws_http_connection *connection_base, size_t increment_size);
+static void s_connection_update_window(struct aws_http_connection *connection_base, uint32_t increment_size);
 static int s_connection_change_settings(
     struct aws_http_connection *connection_base,
     const struct aws_http2_setting *settings_array,
@@ -268,6 +268,7 @@ static void s_stop(
 
 void aws_h2_connection_shutdown_due_to_write_err(struct aws_h2_connection *connection, int error_code) {
     AWS_PRECONDITION(error_code);
+    AWS_PRECONDITION(aws_channel_thread_is_callers_thread(connection->base.channel_slot->channel));
 
     if (connection->thread_data.channel_shutdown_waiting_for_goaway_to_be_written) {
         /* If shutdown is waiting for writes to complete, but writes are now broken,
@@ -2152,28 +2153,29 @@ static bool s_connection_new_requests_allowed(const struct aws_http_connection *
     return new_stream_error_code == 0;
 }
 
-static void s_connection_update_window(struct aws_http_connection *connection_base, size_t increment_size) {
+static void s_connection_update_window(struct aws_http_connection *connection_base, uint32_t increment_size) {
     struct aws_h2_connection *connection = AWS_CONTAINER_OF(connection_base, struct aws_h2_connection, base);
     if (!increment_size) {
+        /* Silently do nothing. */
         return;
     }
     if (!connection_base->manual_window_management) {
-        /* auto-mode, manual update window is not supported */
+        /* auto-mode, manual update window is not supported, silently do nothing with warning log. */
         CONNECTION_LOG(
-            WARN, connection, "Manual window management is off, update window operations are not supported.");
+            DEBUG, connection, "Manual window management is off, update window operations are not supported.");
         return;
     }
-    /* Type cast the increment size here, if overflow happens, we will detect it later, and the frame will be destroyed
-     */
     struct aws_h2_frame *connection_window_update_frame =
-        aws_h2_frame_new_window_update(connection->base.alloc, 0, (uint32_t)increment_size);
+        aws_h2_frame_new_window_update(connection->base.alloc, 0, increment_size);
     if (!connection_window_update_frame) {
         CONNECTION_LOGF(
             ERROR,
             connection,
             "Failed to create WINDOW_UPDATE frame on connection, error %s",
             aws_error_name(aws_last_error()));
-        return;
+        /* OOM should result in a crash. And the increment size is too huge is the only other failure case, which will
+         * result in overflow. */
+        goto overflow;
     }
 
     int err = 0;
@@ -2196,32 +2198,34 @@ static void s_connection_update_window(struct aws_http_connection *connection_ba
         }
         s_unlock_synced_data(connection);
     } /* END CRITICAL SECTION */
+    if (err) {
+        CONNECTION_LOG(
+            ERROR,
+            connection,
+            "The connection's flow-control windows has been incremented beyond 2**31 -1, the max for HTTP/2. The ");
+        aws_h2_frame_destroy(connection_window_update_frame);
+        goto overflow;
+    }
 
     if (cross_thread_work_should_schedule) {
         CONNECTION_LOG(TRACE, connection, "Scheduling cross-thread work task");
         aws_channel_schedule_task_now(connection->base.channel_slot->channel, &connection->cross_thread_work_task);
     }
 
     if (!connection_open) {
-        CONNECTION_LOG(ERROR, connection, "Failed to update connection window, connection is closed or closing.");
-        aws_raise_error(AWS_ERROR_INVALID_STATE);
-        aws_h2_frame_destroy(connection_window_update_frame);
-        return;
-    }
-
-    if (err) {
-        /* The increment_size is still not 100% safe, since we cannot control the incoming data frame. So just
-         * ruled out the value that is obviously wrong values */
-        CONNECTION_LOGF(
-            ERROR,
-            connection,
-            "The increment size is too big for HTTP/2 protocol, max flow-control "
-            "window size is 2147483647. We got %zu, which will cause the flow-control window to exceed the maximum",
-            increment_size);
-        aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
+        /* connection already closed, just do nothing */
         aws_h2_frame_destroy(connection_window_update_frame);
         return;
     }
+    return;
+overflow:
+    /* Shutdown the connection as overflow detected */
+    s_stop(
+        connection,
+        false /*stop_reading*/,
+        false /*stop_writing*/,
+        true /*schedule_shutdown*/,
+        AWS_ERROR_OVERFLOW_DETECTED);
 }
 
 static int s_connection_change_settings(
@@ -2371,7 +2375,9 @@ static int s_connection_send_goaway(
         connection_open = connection->synced_data.is_open;
         if (!connection_open) {
             s_unlock_synced_data(connection);
-            goto closed;
+            CONNECTION_LOG(DEBUG, connection, "Goaway not sent, connection is closed or closing.");
+            aws_mem_release(connection->base.alloc, pending_goaway);
+            goto done;
         }
         was_cross_thread_work_scheduled = connection->synced_data.is_cross_thread_work_task_scheduled;
         connection->synced_data.is_cross_thread_work_task_scheduled = true;
@@ -2392,12 +2398,8 @@ static int s_connection_send_goaway(
         CONNECTION_LOG(TRACE, connection, "Scheduling cross-thread work task");
         aws_channel_schedule_task_now(connection->base.channel_slot->channel, &connection->cross_thread_work_task);
     }
+done:
     return AWS_OP_SUCCESS;
-
-closed:
-    CONNECTION_LOG(ERROR, connection, "Failed to send goaway, connection is closed or closing.");
-    aws_mem_release(connection->base.alloc, pending_goaway);
-    return aws_raise_error(AWS_ERROR_INVALID_STATE);
 }
 
 static void s_get_settings_general(
diff --git a/source/h2_stream.c b/source/h2_stream.c
diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt
diff --git a/tests/test_h2_client.c b/tests/test_h2_client.c
