From ec05a261ca2285ce6ac12194bc5ef826fdfff883 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Fri, 3 May 2019 16:02:43 -0700 Subject: [PATCH 01/14] WIP --- include/aws/http/connection_manager.h | 62 +++++ source/connection_manager.c | 314 ++++++++++++++++++++++++++ tests/CMakeLists.txt | 2 + tests/test_connection_manager.c | 183 +++++++++++++++ 4 files changed, 561 insertions(+) create mode 100644 include/aws/http/connection_manager.h create mode 100644 source/connection_manager.c create mode 100644 tests/test_connection_manager.c diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h new file mode 100644 index 000000000..95bee57e4 --- /dev/null +++ b/include/aws/http/connection_manager.h @@ -0,0 +1,62 @@ +#ifndef AWS_HTTP_CONNECTION_MANAGER_H +#define AWS_HTTP_CONNECTION_MANAGER_H + +/* + * Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +#include + +#include + +struct aws_client_bootstrap; +struct aws_http_connection; +struct aws_http_connection_manager; +struct aws_socket_options; +struct aws_tls_connection_options; + +struct aws_http_connection_manager_options { + struct aws_client_bootstrap *bootstrap; + size_t initial_window_size; + struct aws_socket_options *socket_options; + struct aws_tls_connection_options *tls_connection_options; + struct aws_byte_cursor host; + uint16_t port; + size_t max_connections; +}; + +typedef int (acquire_connection_callback_fn)(struct aws_http_connection *connection, void *user_data, int result); + + +AWS_EXTERN_C_BEGIN + +AWS_HTTP_API +void aws_http_connection_manager_release(struct aws_http_connection_manager *manager); + +AWS_HTTP_API +void aws_http_connection_manager_acquire(struct aws_http_connection_manager *manager); + +AWS_HTTP_API +struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_allocator *allocator, struct aws_http_connection_manager_options *options); + +AWS_HTTP_API +int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, acquire_connection_callback_fn *callback, void *user_data); + +AWS_HTTP_API +int aws_http_connection_manager_release_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection); + +AWS_EXTERN_C_END + +#endif /* AWS_HTTP_CONNECTION_MANAGER_H */ + diff --git a/source/connection_manager.c b/source/connection_manager.c new file mode 100644 index 000000000..f02a84948 --- /dev/null +++ b/source/connection_manager.c @@ -0,0 +1,314 @@ +/* + * Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +enum aws_connection_return_type { + AWS_CRT_CONNECTED, + AWS_CRT_RELEASED +}; + +struct aws_http_connection_manager { + struct aws_allocator *allocator; + struct aws_mutex lock; + struct aws_hash_table connections; + struct aws_linked_list pending_acquisitions; + size_t pending_connection_count; + struct aws_client_bootstrap *bootstrap; + size_t initial_window_size; + struct aws_socket_options socket_options; + struct aws_tls_connection_options tls_connection_options; + struct aws_string *host; + uint16_t port; + size_t max_connections; + size_t vended_connection_count; + struct aws_atomic_var ref_count; +}; + +struct aws_pending_acquisition { + struct aws_linked_list_node node; + acquire_connection_callback_fn *callback; + void *user_data; +}; + +static int s_connection_cleanup(void *context, struct aws_hash_element *element) { + (void)context; + + struct aws_http_connection *connection = (void *)element->key; + aws_http_connection_close(connection); + + return AWS_OP_SUCCESS; +} + +static void s_aws_http_connection_manager_destroy(struct aws_http_connection_manager *manager) { + if (manager == NULL) { + return; + } + + aws_hash_table_foreach(&manager->connections, s_connection_cleanup, NULL); + aws_hash_table_clean_up(&manager->connections); + + while (!aws_linked_list_empty(&manager->pending_acquisitions)) { + struct aws_linked_list_node *node = aws_linked_list_pop_front(&manager->pending_acquisitions); + struct aws_pending_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_pending_acquisition, node); + + pending_acquisition->callback(NULL, pending_acquisition->user_data, AWS_OP_ERR); + aws_mem_release(manager->allocator, pending_acquisition); + } + + aws_string_destroy(manager->host); + aws_tls_connection_options_clean_up(&manager->tls_connection_options); + aws_mutex_clean_up(&manager->lock); + + aws_mem_release(manager->allocator, manager); +} + +void aws_http_connection_manager_acquire(struct aws_http_connection_manager *manager) { + aws_atomic_fetch_add(&manager->ref_count, 1); +} + +void aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { + size_t old_value = aws_atomic_fetch_sub(&manager->ref_count, 1); + if (old_value == 1) { + s_aws_http_connection_manager_destroy(manager); + } +} + +struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_allocator *allocator, struct aws_http_connection_manager_options *options) { + assert(options); + assert(options->socket_options); + assert(options->max_connections > 0); + + struct aws_http_connection_manager *manager = aws_mem_acquire(allocator, sizeof(struct aws_http_connection_manager)); + if (manager == NULL) { + return NULL; + } + + AWS_ZERO_STRUCT(*manager); + manager->allocator = allocator; + + if (aws_mutex_init(&manager->lock)) { + goto on_error; + } + + if (aws_hash_table_init(&manager->connections, allocator, 2, aws_hash_ptr, aws_ptr_eq, NULL, NULL)) { + goto on_error; + } + + aws_linked_list_init(&manager->pending_acquisitions); + + manager->host = aws_string_new_from_array(allocator, options->host.ptr, options->host.len); + if (manager->host == NULL) { + goto on_error; + } + + if (options->tls_connection_options && aws_tls_connection_options_copy(&manager->tls_connection_options, options->tls_connection_options)) { + goto on_error; + } + + manager->initial_window_size = options->initial_window_size; + manager->port = options->port; + manager->max_connections = options->max_connections; + manager->socket_options = *options->socket_options; + manager->bootstrap = options->bootstrap; + + aws_atomic_store_int(&manager->ref_count, 1); + + return manager; + +on_error: + + s_aws_http_connection_manager_destroy(manager); + + return NULL; +} + +static void s_aws_http_connection_manager_add_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection, enum aws_connection_return_type return_type); + +static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_connection *connection, int error_code, void *user_data) { + (void)error_code; + + struct aws_http_connection_manager *manager = user_data; + + if (connection != NULL) { + s_aws_http_connection_manager_add_connection(manager, connection, AWS_CRT_CONNECTED); + return; + } + + aws_mutex_lock(&manager->lock); + + // TODO: implement ??; + + aws_mutex_unlock(&manager->lock); +} + +static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http_connection *connection, int error_code, void *user_data) { + (void)connection; + (void)error_code; + (void)user_data; + + struct aws_http_connection_manager *manager = user_data; + aws_mutex_lock(&manager->lock); + + aws_hash_table_remove(&manager->connections, connection, NULL, NULL); + + aws_mutex_unlock(&manager->lock); +} + +static int s_aws_http_connection_manager_new_connection(struct aws_http_connection_manager *connection_manager) { + assert(aws_hash_table_get_entry_count(&connection_manager->connections) == 0); + + if (connection_manager->vended_connection_count + connection_manager->pending_connection_count >= connection_manager->max_connections) { + return AWS_OP_SUCCESS; + } + + struct aws_http_client_connection_options options; + AWS_ZERO_STRUCT(options); + options.self_size = sizeof(struct aws_http_client_connection_options); + options.bootstrap = connection_manager->bootstrap; + options.tls_options = &connection_manager->tls_connection_options; + options.allocator = connection_manager->allocator; + options.user_data = connection_manager; + options.host_name = aws_byte_cursor_from_string(connection_manager->host); + options.port = connection_manager->port; + options.initial_window_size = connection_manager->initial_window_size; + options.socket_options = &connection_manager->socket_options; + options.on_setup = s_aws_http_connection_manager_on_connection_setup; + options.on_shutdown = s_aws_http_connection_manager_on_connection_shutdown; + + if (aws_http_client_connect(&options)) + { + return AWS_OP_ERR; + } + + ++connection_manager->pending_connection_count; + + return AWS_OP_SUCCESS; +} + +static void s_aws_http_connection_manager_pump_acquisitions(struct aws_http_connection_manager *connection_manager) { + if (!aws_linked_list_empty(&connection_manager->pending_acquisitions)) { + struct aws_linked_list_node *node = aws_linked_list_front(&connection_manager->pending_acquisitions); + struct aws_pending_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_pending_acquisition, node); + + if (s_aws_http_connection_manager_new_connection(connection_manager)) { + + aws_linked_list_pop_front(&connection_manager->pending_acquisitions); + pending_acquisition->callback(NULL, pending_acquisition->user_data, AWS_OP_ERR); + aws_mem_release(connection_manager->allocator, pending_acquisition); + } + } +} + +void s_aws_http_connection_manager_add_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection, enum aws_connection_return_type return_type) { + assert(connection); + + aws_mutex_lock(&connection_manager->lock); + + if (return_type == AWS_CRT_CONNECTED) { + assert(connection_manager->pending_connection_count > 0); + + --connection_manager->pending_connection_count; + } else if (return_type == AWS_CRT_RELEASED) { + assert(connection_manager->vended_connection_count > 0); + + --connection_manager->vended_connection_count; + } + + if (!aws_http_connection_is_open(connection)) { + s_aws_http_connection_manager_pump_acquisitions(connection_manager); + goto done; + } + + if (aws_linked_list_empty(&connection_manager->pending_acquisitions)) { + if (aws_hash_table_put(&connection_manager->connections, &connection, NULL, NULL)) { + aws_http_connection_close(connection); + } + + goto done; + } + + struct aws_linked_list_node *node = aws_linked_list_pop_front(&connection_manager->pending_acquisitions); + struct aws_pending_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_pending_acquisition, node); + + pending_acquisition->callback(connection, pending_acquisition->user_data, AWS_OP_SUCCESS); + aws_mem_release(connection_manager->allocator, pending_acquisition); + + ++connection_manager->vended_connection_count; + +done: + + aws_mutex_unlock(&connection_manager->lock); +} + +int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, acquire_connection_callback_fn *callback, void *user_data) { + int result = AWS_OP_ERR; + + aws_mutex_lock(&connection_manager->lock); + + size_t available_connection_count = aws_hash_table_get_entry_count(&connection_manager->connections); + if (available_connection_count > 0) { + struct aws_hash_iter iter = aws_hash_iter_begin(&connection_manager->connections); + struct aws_http_connection *connection = (void *) iter.element.key; + + aws_hash_iter_delete(&iter, false); + + ++connection_manager->vended_connection_count; + result = AWS_OP_SUCCESS; + callback(connection, user_data, AWS_OP_SUCCESS); + + goto done; + } + + struct aws_pending_acquisition *pending_connection = aws_mem_acquire(connection_manager->allocator, sizeof(struct aws_pending_acquisition)); + if (pending_connection == NULL) { + callback(NULL, user_data, AWS_OP_ERR); + goto done; + } + + if (s_aws_http_connection_manager_new_connection(connection_manager)) { + aws_mem_release(connection_manager->allocator, pending_connection); + callback(NULL, user_data, AWS_OP_ERR); + goto done; + } + + AWS_ZERO_STRUCT(*pending_connection); + + pending_connection->callback = callback; + pending_connection->user_data = user_data; + + aws_linked_list_push_back(&connection_manager->pending_acquisitions, &pending_connection->node); + +done: + + aws_mutex_unlock(&connection_manager->lock); + + return result; +} + +int aws_http_connection_manager_release_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection) { + s_aws_http_connection_manager_add_connection(connection_manager, connection, AWS_CRT_RELEASED); + + return AWS_OP_SUCCESS; +} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index fcad8807a..105a2e9df 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -85,6 +85,8 @@ add_test_case(websocket_encoder_fragmented_message) add_test_case(websocket_encoder_fragmentation_failure_checks) add_test_case(websocket_encoder_payload_callback_can_fail_encoder) +add_test_case(test_connection_manager_setup_shutdown) + set(TEST_BINARY_NAME ${CMAKE_PROJECT_NAME}-tests) generate_test_driver(${TEST_BINARY_NAME}) diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c new file mode 100644 index 000000000..f2c10dd89 --- /dev/null +++ b/tests/test_connection_manager.c @@ -0,0 +1,183 @@ +/* + * Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct cm_tester_options { + struct aws_allocator *allocator; + size_t max_connections; +}; + +struct cm_tester { + struct aws_allocator *allocator; + struct aws_event_loop_group event_loop_group; + struct aws_host_resolver host_resolver; + + struct aws_client_bootstrap *client_bootstrap; + + struct aws_http_connection_manager *connection_manager; + + struct aws_tls_ctx *tls_ctx; + struct aws_tls_ctx_options tls_ctx_options; + struct aws_tls_connection_options tls_connection_options; + + struct aws_mutex wait_lock; + struct aws_condition_variable wait_cvar; + + int wait_result; +}; + +#ifdef NEVER +static void s_tester_on_client_connection_setup( + struct aws_http_connection *connection, + int error_code, + void *user_data) { + + struct tester *tester = user_data; + AWS_FATAL_ASSERT(aws_mutex_lock(&tester->wait_lock) == AWS_OP_SUCCESS); + + if (error_code) { + tester->wait_result = error_code; + goto done; + } + + tester->client_connection = connection; + done: + AWS_FATAL_ASSERT(aws_mutex_unlock(&tester->wait_lock) == AWS_OP_SUCCESS); + aws_condition_variable_notify_one(&tester->wait_cvar); +} + +static void s_tester_on_client_connection_shutdown( + struct aws_http_connection *connection, + int error_code, + void *user_data) { + + (void)connection; + (void)error_code; + struct tester *tester = user_data; + AWS_FATAL_ASSERT(aws_mutex_lock(&tester->wait_lock) == AWS_OP_SUCCESS); + + tester->client_connection_is_shutdown = true; + + AWS_FATAL_ASSERT(aws_mutex_unlock(&tester->wait_lock) == AWS_OP_SUCCESS); + aws_condition_variable_notify_one(&tester->wait_cvar); +} +#endif + +int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options) { + AWS_ZERO_STRUCT(*tester); + + aws_tls_init_static_state(options->allocator); + aws_http_library_init(options->allocator); + aws_load_error_strings(); + aws_io_load_error_strings(); + + tester->allocator = options->allocator; + + if (aws_mutex_init(&tester->wait_lock)) { + return AWS_OP_ERR; + } + + if (aws_condition_variable_init(&tester->wait_cvar)) { + return AWS_OP_ERR; + } + + ASSERT_SUCCESS(aws_event_loop_group_default_init(&tester->event_loop_group, tester->allocator, 1)); + ASSERT_SUCCESS(aws_host_resolver_init_default(&tester->host_resolver, tester->allocator, 8, &tester->event_loop_group)); + tester->client_bootstrap = + aws_client_bootstrap_new(tester->allocator, &tester->event_loop_group, &tester->host_resolver, NULL); + ASSERT_NOT_NULL(tester->client_bootstrap); + + struct aws_socket_options socket_options = { + .type = AWS_SOCKET_STREAM, + .domain = AWS_SOCKET_IPV4, + .connect_timeout_ms = + (uint32_t)aws_timestamp_convert(60, AWS_TIMESTAMP_SECS, AWS_TIMESTAMP_MILLIS, NULL), + }; + + aws_tls_ctx_options_init_default_client(&tester->tls_ctx_options, options->allocator); + + tester->tls_ctx = aws_tls_client_ctx_new(options->allocator, &tester->tls_ctx_options); + ASSERT_NOT_NULL(tester->tls_ctx); + + aws_tls_connection_options_init_from_ctx(&tester->tls_connection_options, tester->tls_ctx); + + struct aws_http_connection_manager_options cm_options = { + .bootstrap = tester->client_bootstrap, + .initial_window_size = SIZE_MAX, + .socket_options = &socket_options, + .tls_connection_options = &tester->tls_connection_options, + .host = aws_byte_cursor_from_c_str("https://s3.amazonaws.com/"), + .port = 443, + .max_connections = options->max_connections + }; + + tester->connection_manager = aws_http_connection_manager_new(tester->allocator, &cm_options); + ASSERT_NOT_NULL(tester->connection_manager); + + return AWS_OP_SUCCESS; +} + +void s_cm_tester_clean_up(struct cm_tester *tester) { + if (tester == NULL) { + return; + } + + aws_http_connection_manager_release(tester->connection_manager); + + aws_client_bootstrap_release(tester->client_bootstrap); + + aws_host_resolver_clean_up(&tester->host_resolver); + aws_event_loop_group_clean_up(&tester->event_loop_group); + + aws_tls_ctx_options_clean_up(&tester->tls_ctx_options); + aws_tls_connection_options_clean_up(&tester->tls_connection_options); + aws_tls_ctx_destroy(tester->tls_ctx); + + aws_mutex_clean_up(&tester->wait_lock); + aws_condition_variable_clean_up(&tester->wait_cvar); + + aws_http_library_clean_up(); + aws_tls_clean_up_static_state(); +} + +static int s_test_connection_manager_setup_shutdown(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct cm_tester_options options = { + .allocator = allocator, + .max_connections = 5 + }; + + struct cm_tester tester; + ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + + s_cm_tester_clean_up(&tester); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_connection_manager_setup_shutdown, s_test_connection_manager_setup_shutdown); \ No newline at end of file From c90fd321ecaf17796803eb4a4f942132fcfacd7e Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Wed, 8 May 2019 11:33:38 -0700 Subject: [PATCH 02/14] First draft of connection manager --- include/aws/http/connection_manager.h | 32 +++- source/connection_manager.c | 258 ++++++++++++++++++++------ 2 files changed, 226 insertions(+), 64 deletions(-) diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h index 95bee57e4..f19c7651f 100644 --- a/include/aws/http/connection_manager.h +++ b/include/aws/http/connection_manager.h @@ -19,13 +19,19 @@ #include #include +#include struct aws_client_bootstrap; -struct aws_http_connection; struct aws_http_connection_manager; struct aws_socket_options; struct aws_tls_connection_options; +/* + * Connection manager configuration struct. + * + * Contains all of the configuration needed to create an http connection as well as + * the maximum number of connections to ever have in existence. + */ struct aws_http_connection_manager_options { struct aws_client_bootstrap *bootstrap; size_t initial_window_size; @@ -36,23 +42,35 @@ struct aws_http_connection_manager_options { size_t max_connections; }; -typedef int (acquire_connection_callback_fn)(struct aws_http_connection *connection, void *user_data, int result); - - AWS_EXTERN_C_BEGIN +/* + * Connection managers are ref counted. Adds one external ref to the manager. + */ AWS_HTTP_API -void aws_http_connection_manager_release(struct aws_http_connection_manager *manager); +void aws_http_connection_manager_acquire(struct aws_http_connection_manager *manager); +/* + * Connection managers are ref counted. Removes one external ref from the manager. + */ AWS_HTTP_API -void aws_http_connection_manager_acquire(struct aws_http_connection_manager *manager); +void aws_http_connection_manager_release(struct aws_http_connection_manager *manager); +/* + * Creates a new connection manager with the supplied configuration options. + */ AWS_HTTP_API struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_allocator *allocator, struct aws_http_connection_manager_options *options); +/* + * Requests a connection from the manager + */ AWS_HTTP_API -int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, acquire_connection_callback_fn *callback, void *user_data); +int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, aws_http_on_client_connection_setup_fn *callback, void *user_data); +/* + * Returns a connection back to the manager + */ AWS_HTTP_API int aws_http_connection_manager_release_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection); diff --git a/source/connection_manager.c b/source/connection_manager.c index f02a84948..47654a6ce 100644 --- a/source/connection_manager.c +++ b/source/connection_manager.c @@ -24,34 +24,122 @@ #include #include -enum aws_connection_return_type { - AWS_CRT_CONNECTED, - AWS_CRT_RELEASED +enum aws_http_connection_accept_type { + AWS_CAT_NEWLY_CONNECTED, + AWS_CAT_RELEASED }; struct aws_http_connection_manager { struct aws_allocator *allocator; + + /* + * Controls access to all mutable state on the connection manager + */ struct aws_mutex lock; + + /* + * The set of all available ready-to-be-used connections + */ struct aws_hash_table connections; + + /* + * The set of all incomplete connection acquisition requests + */ struct aws_linked_list pending_acquisitions; - size_t pending_connection_count; + + /* + * The number of all incomplete connection acquisition requests. So + * that we don't have compute the size of a linked list every time. + */ + size_t pending_acquisition_count; + + /* + * The number of pending new connection requests we have outstanding to the http + * layer. Each pending new connection requests adds one to the connection manager's + * overall ref count. Each resolved request subtracts one. + */ + size_t pending_connects_count; + + /* + * The number of connections currently being used by external users. + */ + size_t vended_connection_count; + + /* + * All the options needed to create an http connection + */ struct aws_client_bootstrap *bootstrap; size_t initial_window_size; struct aws_socket_options socket_options; struct aws_tls_connection_options tls_connection_options; struct aws_string *host; uint16_t port; + + /* + * The maximum number of connections this manager should ever have at once. + */ size_t max_connections; - size_t vended_connection_count; + + /* + * Lifecycle tracking for the connection manager. Starts at 1. + * + * value = # external refs + # pending connects + */ struct aws_atomic_var ref_count; }; -struct aws_pending_acquisition { +/* + * A struct that functions as both the pending acquisition tracker and the about-to-complete data. + * + * The list in the connection manager is the set of all acquisition requests that we haven't resolved. + * + * In order to make sure we never invoke callbacks while holding the manager's lock, in a number of places + * we build a list of one or more acquisitions to complete while holding the lock. Once the lock is released + * we complete all the acquisitions in the list using the data within the struct (hence why we have + * connection and result members). + */ +struct aws_http_connection_acquisition { struct aws_linked_list_node node; - acquire_connection_callback_fn *callback; + aws_http_on_client_connection_setup_fn *callback; void *user_data; + struct aws_http_connection *connection; + int result; }; +/* + * Only call this outside the scope of the connection manager's lock + */ +static void s_aws_http_connection_manager_complete_acquisitions(struct aws_linked_list *acquisitions, struct aws_allocator *allocator) { + while (!aws_linked_list_empty(acquisitions)) { + struct aws_linked_list_node *node = aws_linked_list_pop_front(acquisitions); + struct aws_http_connection_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_http_connection_acquisition, node); + + pending_acquisition->callback(pending_acquisition->connection, pending_acquisition->result, pending_acquisition->user_data); + aws_mem_release(allocator, pending_acquisition); + } +} + +/* + * Moves the first pending connection acquisition into a list. Call this while holding the lock to + * build the set of callbacks to be completed once the lock is released. + * + * If this was a successful acquisition then connection is non-null + * If this was a failed acquisition then connection is null + */ +static void s_aws_http_connection_manager_move_front_acquisition(struct aws_http_connection_manager *manager, struct aws_http_connection *connection, int result, struct aws_linked_list *output_list) { + assert(!aws_linked_list_empty(&manager->pending_acquisitions)); + assert(manager->pending_acquisition_count > 0); + + struct aws_linked_list_node *node = aws_linked_list_pop_front(&manager->pending_acquisitions); + --manager->pending_acquisition_count; + + struct aws_http_connection_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_http_connection_acquisition, node); + pending_acquisition->connection = connection; + pending_acquisition->result = result; + + aws_linked_list_push_back(output_list, node); +} + static int s_connection_cleanup(void *context, struct aws_hash_element *element) { (void)context; @@ -66,16 +154,13 @@ static void s_aws_http_connection_manager_destroy(struct aws_http_connection_man return; } + assert(manager->pending_connects_count == 0); + aws_hash_table_foreach(&manager->connections, s_connection_cleanup, NULL); aws_hash_table_clean_up(&manager->connections); - while (!aws_linked_list_empty(&manager->pending_acquisitions)) { - struct aws_linked_list_node *node = aws_linked_list_pop_front(&manager->pending_acquisitions); - struct aws_pending_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_pending_acquisition, node); - - pending_acquisition->callback(NULL, pending_acquisition->user_data, AWS_OP_ERR); - aws_mem_release(manager->allocator, pending_acquisition); - } + s_aws_http_connection_manager_complete_acquisitions(&manager->pending_acquisitions, manager->allocator); + manager->pending_acquisition_count = 0; aws_string_destroy(manager->host); aws_tls_connection_options_clean_up(&manager->tls_connection_options); @@ -84,17 +169,6 @@ static void s_aws_http_connection_manager_destroy(struct aws_http_connection_man aws_mem_release(manager->allocator, manager); } -void aws_http_connection_manager_acquire(struct aws_http_connection_manager *manager) { - aws_atomic_fetch_add(&manager->ref_count, 1); -} - -void aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { - size_t old_value = aws_atomic_fetch_sub(&manager->ref_count, 1); - if (old_value == 1) { - s_aws_http_connection_manager_destroy(manager); - } -} - struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_allocator *allocator, struct aws_http_connection_manager_options *options) { assert(options); assert(options->socket_options); @@ -144,7 +218,20 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_a return NULL; } -static void s_aws_http_connection_manager_add_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection, enum aws_connection_return_type return_type); +void aws_http_connection_manager_acquire(struct aws_http_connection_manager *manager) { + aws_atomic_fetch_add(&manager->ref_count, 1); +} + +void aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { + size_t old_value = aws_atomic_fetch_sub(&manager->ref_count, 1); + if (old_value == 1) { + s_aws_http_connection_manager_destroy(manager); + } +} + +static void s_aws_http_connection_manager_accept_connection(struct aws_http_connection_manager *connection_manager, + struct aws_http_connection *connection, + enum aws_http_connection_accept_type return_type); static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_connection *connection, int error_code, void *user_data) { (void)error_code; @@ -152,15 +239,36 @@ static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_co struct aws_http_connection_manager *manager = user_data; if (connection != NULL) { - s_aws_http_connection_manager_add_connection(manager, connection, AWS_CRT_CONNECTED); + s_aws_http_connection_manager_accept_connection(manager, connection, AWS_CAT_NEWLY_CONNECTED); return; } aws_mutex_lock(&manager->lock); - // TODO: implement ??; + assert(manager->pending_connects_count > 0); + --manager->pending_connects_count; + + /* + * A nice behavioral optimization. If we failed to connect now, then we're more likely to fail in the near-future + * as well. So if we have an excess of pending acquisitions (beyond the number of pending connects), let's fail + * all of the excess. + */ + struct aws_linked_list trimmed_acquisitions; + aws_linked_list_init(&trimmed_acquisitions); + + while(manager->pending_acquisition_count > manager->pending_connects_count) { + s_aws_http_connection_manager_move_front_acquisition(manager, NULL, error_code, &trimmed_acquisitions); + } aws_mutex_unlock(&manager->lock); + + s_aws_http_connection_manager_complete_acquisitions(&trimmed_acquisitions, manager->allocator); + + /* + * Normally paired with pending_connects_count side effect, but let's delay this until the very end on + * the offchance that someone released the last external ref while there was still a pending connect. + */ + aws_http_connection_manager_release(manager); } static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http_connection *connection, int error_code, void *user_data) { @@ -179,7 +287,10 @@ static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http static int s_aws_http_connection_manager_new_connection(struct aws_http_connection_manager *connection_manager) { assert(aws_hash_table_get_entry_count(&connection_manager->connections) == 0); - if (connection_manager->vended_connection_count + connection_manager->pending_connection_count >= connection_manager->max_connections) { + /* + * Don't create too many + */ + if (connection_manager->vended_connection_count + connection_manager->pending_connects_count >= connection_manager->max_connections) { return AWS_OP_SUCCESS; } @@ -202,42 +313,51 @@ static int s_aws_http_connection_manager_new_connection(struct aws_http_connecti return AWS_OP_ERR; } - ++connection_manager->pending_connection_count; + ++connection_manager->pending_connects_count; + aws_http_connection_manager_acquire(connection_manager); return AWS_OP_SUCCESS; } -static void s_aws_http_connection_manager_pump_acquisitions(struct aws_http_connection_manager *connection_manager) { +/* + * A utility function that, if there's at least one pending acquisition, attempts to create a new connection for it. + */ +static void s_aws_http_connection_manager_pump_acquisitions(struct aws_http_connection_manager *connection_manager, struct aws_linked_list *to_complete_acquisitions) { if (!aws_linked_list_empty(&connection_manager->pending_acquisitions)) { - struct aws_linked_list_node *node = aws_linked_list_front(&connection_manager->pending_acquisitions); - struct aws_pending_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_pending_acquisition, node); - if (s_aws_http_connection_manager_new_connection(connection_manager)) { - - aws_linked_list_pop_front(&connection_manager->pending_acquisitions); - pending_acquisition->callback(NULL, pending_acquisition->user_data, AWS_OP_ERR); - aws_mem_release(connection_manager->allocator, pending_acquisition); + s_aws_http_connection_manager_move_front_acquisition(connection_manager, NULL, aws_last_error(), to_complete_acquisitions); } } } -void s_aws_http_connection_manager_add_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection, enum aws_connection_return_type return_type) { +/* + * Shared implementation for + * (1) a new connection comes back from the http layer (AWS_CAT_NEWLY_CONNECTED) + * (2) an old connection is returned by an external user (AWS_CAT_RELEASED) + */ +void s_aws_http_connection_manager_accept_connection(struct aws_http_connection_manager *connection_manager, + struct aws_http_connection *connection, + enum aws_http_connection_accept_type return_type) { assert(connection); - aws_mutex_lock(&connection_manager->lock); + struct aws_linked_list to_complete_acquisitions; + aws_linked_list_init(&to_complete_acquisitions); - if (return_type == AWS_CRT_CONNECTED) { - assert(connection_manager->pending_connection_count > 0); + aws_mutex_lock(&connection_manager->lock); - --connection_manager->pending_connection_count; - } else if (return_type == AWS_CRT_RELEASED) { + if (return_type == AWS_CAT_NEWLY_CONNECTED) { + assert(connection_manager->pending_connects_count > 0); + --connection_manager->pending_connects_count; + } else if (return_type == AWS_CAT_RELEASED) { assert(connection_manager->vended_connection_count > 0); - --connection_manager->vended_connection_count; } + /* + * If the connection has expired, we should try to replace it with a new one. + */ if (!aws_http_connection_is_open(connection)) { - s_aws_http_connection_manager_pump_acquisitions(connection_manager); + s_aws_http_connection_manager_pump_acquisitions(connection_manager, &to_complete_acquisitions); goto done; } @@ -249,24 +369,41 @@ void s_aws_http_connection_manager_add_connection(struct aws_http_connection_man goto done; } - struct aws_linked_list_node *node = aws_linked_list_pop_front(&connection_manager->pending_acquisitions); - struct aws_pending_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_pending_acquisition, node); - - pending_acquisition->callback(connection, pending_acquisition->user_data, AWS_OP_SUCCESS); - aws_mem_release(connection_manager->allocator, pending_acquisition); + s_aws_http_connection_manager_move_front_acquisition(connection_manager, connection, AWS_ERROR_SUCCESS, &to_complete_acquisitions); ++connection_manager->vended_connection_count; done: aws_mutex_unlock(&connection_manager->lock); + + s_aws_http_connection_manager_complete_acquisitions(&to_complete_acquisitions, connection_manager->allocator); + + /* + * Delay until the very end just in case someone dropped the last external ref while there's a pending connect + */ + if (return_type == AWS_CAT_NEWLY_CONNECTED) { + aws_http_connection_manager_release(connection_manager); + } } -int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, acquire_connection_callback_fn *callback, void *user_data) { +/* + * Unlike the other functions, using the to_complete_acquisitions list doesn't make sense here because many + * of the completion points do not have an aws_http_connection_acquisition structure filled out. So just + * do it with local variables (make_callback, callback_connection). + */ +int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, aws_http_on_client_connection_setup_fn *callback, void *user_data) { int result = AWS_OP_ERR; + bool make_callback = false; + struct aws_http_connection *callback_connection = NULL; + aws_mutex_lock(&connection_manager->lock); + /* + * Future: possibly worth evaluating whether LIFO-ordering ready connections gives a performance improvement (cache-wise) + * Would require moving from hash table to a different data structure + */ size_t available_connection_count = aws_hash_table_get_entry_count(&connection_manager->connections); if (available_connection_count > 0) { struct aws_hash_iter iter = aws_hash_iter_begin(&connection_manager->connections); @@ -276,20 +413,22 @@ int aws_http_connection_manager_acquire_connection(struct aws_http_connection_ma ++connection_manager->vended_connection_count; result = AWS_OP_SUCCESS; - callback(connection, user_data, AWS_OP_SUCCESS); + + make_callback = true; + callback_connection = connection; goto done; } - struct aws_pending_acquisition *pending_connection = aws_mem_acquire(connection_manager->allocator, sizeof(struct aws_pending_acquisition)); + struct aws_http_connection_acquisition *pending_connection = aws_mem_acquire(connection_manager->allocator, sizeof(struct aws_http_connection_acquisition)); if (pending_connection == NULL) { - callback(NULL, user_data, AWS_OP_ERR); + make_callback = true; goto done; } if (s_aws_http_connection_manager_new_connection(connection_manager)) { aws_mem_release(connection_manager->allocator, pending_connection); - callback(NULL, user_data, AWS_OP_ERR); + make_callback = true; goto done; } @@ -299,16 +438,21 @@ int aws_http_connection_manager_acquire_connection(struct aws_http_connection_ma pending_connection->user_data = user_data; aws_linked_list_push_back(&connection_manager->pending_acquisitions, &pending_connection->node); + ++connection_manager->pending_acquisition_count; done: aws_mutex_unlock(&connection_manager->lock); + if (make_callback) { + callback(callback_connection, callback_connection != NULL ? AWS_ERROR_SUCCESS : aws_last_error(), user_data); + } + return result; } int aws_http_connection_manager_release_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection) { - s_aws_http_connection_manager_add_connection(connection_manager, connection, AWS_CRT_RELEASED); + s_aws_http_connection_manager_accept_connection(connection_manager, connection, AWS_CAT_RELEASED); return AWS_OP_SUCCESS; } From f08703b9588f859d04560d2f6c4f07c5a0657e7d Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Thu, 9 May 2019 16:07:51 -0700 Subject: [PATCH 03/14] WIP --- include/aws/http/connection_manager.h | 2 + .../connection_manager_function_table.h | 38 ++ source/connection_manager.c | 372 ++++++++++-------- tests/test_connection_manager.c | 97 +++-- tests/test_websocket_encoder.c | 2 +- 5 files changed, 298 insertions(+), 213 deletions(-) create mode 100644 include/aws/http/private/connection_manager_function_table.h diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h index f19c7651f..0e5c1b7c6 100644 --- a/include/aws/http/connection_manager.h +++ b/include/aws/http/connection_manager.h @@ -25,6 +25,7 @@ struct aws_client_bootstrap; struct aws_http_connection_manager; struct aws_socket_options; struct aws_tls_connection_options; +struct aws_http_connection_manager_mocks; /* * Connection manager configuration struct. @@ -40,6 +41,7 @@ struct aws_http_connection_manager_options { struct aws_byte_cursor host; uint16_t port; size_t max_connections; + struct aws_http_connection_manager_function_table *mocks; }; AWS_EXTERN_C_BEGIN diff --git a/include/aws/http/private/connection_manager_function_table.h b/include/aws/http/private/connection_manager_function_table.h new file mode 100644 index 000000000..317e23b4e --- /dev/null +++ b/include/aws/http/private/connection_manager_function_table.h @@ -0,0 +1,38 @@ +#ifndef AWS_HTTP_CONNECTION_MANAGER_FUNCTION_TABLE_H +#define AWS_HTTP_CONNECTION_MANAGER_FUNCTION_TABLE_H + +/* + * Copyright 2010-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +#include + +#include + +typedef int (aws_http_connection_manager_create_connection_fn)(const struct aws_http_client_connection_options *options); +typedef void (aws_http_connection_manager_close_connection_fn)(struct aws_http_connection *connection); +typedef void (aws_http_connection_manager_release_connection_fn)(struct aws_http_connection *connection); + +struct aws_http_connection_manager_function_table { + aws_http_connection_manager_create_connection_fn *create_connection; + aws_http_connection_manager_close_connection_fn *close_connection; + aws_http_connection_manager_release_connection_fn *release_connection; +}; + +AWS_STATIC_IMPL +bool aws_http_connection_manager_function_table_is_valid(struct aws_http_connection_manager_function_table *table) { + return table->create_connection && table->close_connection && table->release_connection; +} + +#endif /* AWS_HTTP_CONNECTION_MANAGER_FUNCTION_TABLE_H */ diff --git a/source/connection_manager.c b/source/connection_manager.c index 47654a6ce..1e33e4a03 100644 --- a/source/connection_manager.c +++ b/source/connection_manager.c @@ -23,20 +23,31 @@ #include #include #include +#include -enum aws_http_connection_accept_type { - AWS_CAT_NEWLY_CONNECTED, - AWS_CAT_RELEASED +static struct aws_http_connection_manager_function_table s_default_function_table = { + .create_connection = aws_http_client_connect, + .release_connection = aws_http_connection_release, + .close_connection = aws_http_connection_close +}; + +enum aws_http_connection_manager_state_type { + AWS_HCMST_READY, + AWS_HCMST_SHUTTING_DOWN }; struct aws_http_connection_manager { struct aws_allocator *allocator; + struct aws_http_connection_manager_function_table *functions; + /* * Controls access to all mutable state on the connection manager */ struct aws_mutex lock; + enum aws_http_connection_manager_state_type state; + /* * The set of all available ready-to-be-used connections */ @@ -65,6 +76,8 @@ struct aws_http_connection_manager { */ size_t vended_connection_count; + size_t open_connection_count; + /* * All the options needed to create an http connection */ @@ -83,9 +96,10 @@ struct aws_http_connection_manager { /* * Lifecycle tracking for the connection manager. Starts at 1. * - * value = # external refs + # pending connects + * While state == ready : value = # external refs + # vended connects + * While state == shutting_down : value = # of connection shutdown callbacks not yet invoked */ - struct aws_atomic_var ref_count; + size_t ref_count; }; /* @@ -141,10 +155,10 @@ static void s_aws_http_connection_manager_move_front_acquisition(struct aws_http } static int s_connection_cleanup(void *context, struct aws_hash_element *element) { - (void)context; + struct aws_http_connection_manager *manager = context; struct aws_http_connection *connection = (void *)element->key; - aws_http_connection_close(connection); + manager->functions->release_connection(connection); return AWS_OP_SUCCESS; } @@ -155,13 +169,14 @@ static void s_aws_http_connection_manager_destroy(struct aws_http_connection_man } assert(manager->pending_connects_count == 0); + assert(manager->vended_connection_count == 0); + assert(manager->pending_acquisition_count == 0); + assert(manager->open_connection_count == 0); + assert(aws_linked_list_empty(&manager->pending_acquisitions)); + assert(aws_hash_table_get_entry_count(&manager->connections) == 0); - aws_hash_table_foreach(&manager->connections, s_connection_cleanup, NULL); aws_hash_table_clean_up(&manager->connections); - s_aws_http_connection_manager_complete_acquisitions(&manager->pending_acquisitions, manager->allocator); - manager->pending_acquisition_count = 0; - aws_string_destroy(manager->host); aws_tls_connection_options_clean_up(&manager->tls_connection_options); aws_mutex_clean_up(&manager->lock); @@ -201,13 +216,19 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_a goto on_error; } + manager->state = AWS_HCMST_READY; manager->initial_window_size = options->initial_window_size; manager->port = options->port; manager->max_connections = options->max_connections; manager->socket_options = *options->socket_options; manager->bootstrap = options->bootstrap; + manager->functions = options->mocks; + manager->ref_count = 1; + if (manager->functions == NULL) { + manager->functions = &s_default_function_table; + } - aws_atomic_store_int(&manager->ref_count, 1); + assert(aws_http_connection_manager_function_table_is_valid(manager->functions)); return manager; @@ -219,81 +240,83 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_a } void aws_http_connection_manager_acquire(struct aws_http_connection_manager *manager) { - aws_atomic_fetch_add(&manager->ref_count, 1); + aws_mutex_lock(&manager->lock); + manager->ref_count += 1; + aws_mutex_unlock(&manager->lock); } -void aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { - size_t old_value = aws_atomic_fetch_sub(&manager->ref_count, 1); - if (old_value == 1) { - s_aws_http_connection_manager_destroy(manager); - } -} +enum aws_async_release_result { + AWS_ARR_NONE, + AWS_ARR_START_SHUT_DOWN, + AWS_ARR_DELETE +}; -static void s_aws_http_connection_manager_accept_connection(struct aws_http_connection_manager *connection_manager, - struct aws_http_connection *connection, - enum aws_http_connection_accept_type return_type); +static enum aws_async_release_result s_aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { + AWS_FATAL_ASSERT(manager->ref_count > 0); -static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_connection *connection, int error_code, void *user_data) { - (void)error_code; + enum aws_async_release_result result = AWS_ARR_NONE; + manager->ref_count -= 1; - struct aws_http_connection_manager *manager = user_data; + if (manager->ref_count == 0 && manager->state == AWS_HCMST_READY) { + assert(manager->vended_connection_count == 0); - if (connection != NULL) { - s_aws_http_connection_manager_accept_connection(manager, connection, AWS_CAT_NEWLY_CONNECTED); - return; + manager->state = AWS_HCMST_SHUTTING_DOWN; + manager->ref_count = manager->open_connection_count; + result = manager->ref_count > 0 ? AWS_ARR_START_SHUT_DOWN : AWS_ARR_DELETE; } - aws_mutex_lock(&manager->lock); - - assert(manager->pending_connects_count > 0); - --manager->pending_connects_count; + if (manager->ref_count == 0 && manager->state == AWS_HCMST_SHUTTING_DOWN) { + result = AWS_ARR_DELETE; + } - /* - * A nice behavioral optimization. If we failed to connect now, then we're more likely to fail in the near-future - * as well. So if we have an excess of pending acquisitions (beyond the number of pending connects), let's fail - * all of the excess. - */ - struct aws_linked_list trimmed_acquisitions; - aws_linked_list_init(&trimmed_acquisitions); + return result; +} - while(manager->pending_acquisition_count > manager->pending_connects_count) { - s_aws_http_connection_manager_move_front_acquisition(manager, NULL, error_code, &trimmed_acquisitions); +void aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { + aws_mutex_lock(&manager->lock); + bool start_shutdown_process = s_aws_http_connection_manager_release(manager); + if (start_shutdown_process) { + ??; } - aws_mutex_unlock(&manager->lock); +} - s_aws_http_connection_manager_complete_acquisitions(&trimmed_acquisitions, manager->allocator); - +static void s_aws_http_connection_manager_build_work_order(struct aws_http_connection_manager *connection_manager, + struct aws_linked_list *completions, size_t *new_connections) { /* - * Normally paired with pending_connects_count side effect, but let's delay this until the very end on - * the offchance that someone released the last external ref while there was still a pending connect. + * Step 1 - If there's free connections, complete acquisition requests */ - aws_http_connection_manager_release(manager); -} + while(aws_hash_table_get_entry_count(&connection_manager->connections) > 0 && connection_manager->pending_acquisition_count > 0) { + struct aws_hash_iter iter = aws_hash_iter_begin(&connection_manager->connections); + struct aws_http_connection *connection = (void *) iter.element.key; -static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http_connection *connection, int error_code, void *user_data) { - (void)connection; - (void)error_code; - (void)user_data; + aws_hash_iter_delete(&iter, false); - struct aws_http_connection_manager *manager = user_data; - aws_mutex_lock(&manager->lock); + s_aws_http_connection_manager_move_front_acquisition(connection_manager, connection, AWS_ERROR_SUCCESS, completions); + ++connection_manager->vended_connection_count; + } - aws_hash_table_remove(&manager->connections, connection, NULL, NULL); + /* + * Step 2 - if there's excess pending acquisitions and we have room to make more, make more + */ + if (connection_manager->pending_acquisition_count > connection_manager->pending_connects_count) { + *new_connections = connection_manager->pending_acquisition_count - connection_manager->pending_connects_count; - aws_mutex_unlock(&manager->lock); -} + assert(connection_manager->max_connections >= connection_manager->vended_connection_count + connection_manager->pending_connects_count); + size_t max_new_connections = connection_manager->max_connections - (connection_manager->vended_connection_count + connection_manager->pending_connects_count); -static int s_aws_http_connection_manager_new_connection(struct aws_http_connection_manager *connection_manager) { - assert(aws_hash_table_get_entry_count(&connection_manager->connections) == 0); + if (*new_connections > max_new_connections) { + *new_connections = max_new_connections; + } - /* - * Don't create too many - */ - if (connection_manager->vended_connection_count + connection_manager->pending_connects_count >= connection_manager->max_connections) { - return AWS_OP_SUCCESS; + connection_manager->pending_connects_count += *new_connections; } +} + +static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_connection *connection, int error_code, void *user_data); +static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http_connection *connection, int error_code, void *user_data); +static int s_aws_http_connection_manager_new_connection(struct aws_http_connection_manager *connection_manager) { struct aws_http_client_connection_options options; AWS_ZERO_STRUCT(options); options.self_size = sizeof(struct aws_http_client_connection_options); @@ -308,151 +331,176 @@ static int s_aws_http_connection_manager_new_connection(struct aws_http_connecti options.on_setup = s_aws_http_connection_manager_on_connection_setup; options.on_shutdown = s_aws_http_connection_manager_on_connection_shutdown; - if (aws_http_client_connect(&options)) + if (connection_manager->functions->create_connection(&options)) { return AWS_OP_ERR; } - ++connection_manager->pending_connects_count; - aws_http_connection_manager_acquire(connection_manager); - return AWS_OP_SUCCESS; } -/* - * A utility function that, if there's at least one pending acquisition, attempts to create a new connection for it. - */ -static void s_aws_http_connection_manager_pump_acquisitions(struct aws_http_connection_manager *connection_manager, struct aws_linked_list *to_complete_acquisitions) { - if (!aws_linked_list_empty(&connection_manager->pending_acquisitions)) { - if (s_aws_http_connection_manager_new_connection(connection_manager)) { - s_aws_http_connection_manager_move_front_acquisition(connection_manager, NULL, aws_last_error(), to_complete_acquisitions); + +static void s_aws_http_connection_manager_execute_work_order(struct aws_http_connection_manager *connection_manager, + struct aws_linked_list *completions, + size_t new_connections) { + + int representative_error = 0; + size_t new_connection_failures = 0; + + struct aws_array_list errors; + AWS_ZERO_STRUCT(errors); + + if (!aws_array_list_init_dynamic(&errors, connection_manager->allocator, new_connections, sizeof(int))) { + for (size_t i = 0; i < new_connections; ++i) { + if (s_aws_http_connection_manager_new_connection(connection_manager)) { + representative_error = aws_last_error(); + aws_array_list_push_back(&errors, &representative_error); + } } } -} -/* - * Shared implementation for - * (1) a new connection comes back from the http layer (AWS_CAT_NEWLY_CONNECTED) - * (2) an old connection is returned by an external user (AWS_CAT_RELEASED) - */ -void s_aws_http_connection_manager_accept_connection(struct aws_http_connection_manager *connection_manager, - struct aws_http_connection *connection, - enum aws_http_connection_accept_type return_type) { - assert(connection); + if (new_connection_failures > 0) { + aws_mutex_lock(&connection_manager->lock); - struct aws_linked_list to_complete_acquisitions; - aws_linked_list_init(&to_complete_acquisitions); + connection_manager->pending_connects_count -= new_connection_failures; + for (size_t i = 0; + i < new_connection_failures && !aws_linked_list_empty(&connection_manager->pending_acquisitions); ++i) { + int error = representative_error; + if (i < aws_array_list_length(&errors)) { + aws_array_list_get_at(&errors, &error, i); + } - aws_mutex_lock(&connection_manager->lock); + s_aws_http_connection_manager_move_front_acquisition(connection_manager, NULL, error, completions); + } - if (return_type == AWS_CAT_NEWLY_CONNECTED) { - assert(connection_manager->pending_connects_count > 0); - --connection_manager->pending_connects_count; - } else if (return_type == AWS_CAT_RELEASED) { - assert(connection_manager->vended_connection_count > 0); - --connection_manager->vended_connection_count; + aws_mutex_unlock(&connection_manager->lock); } - /* - * If the connection has expired, we should try to replace it with a new one. - */ - if (!aws_http_connection_is_open(connection)) { - s_aws_http_connection_manager_pump_acquisitions(connection_manager, &to_complete_acquisitions); - goto done; - } + s_aws_http_connection_manager_complete_acquisitions(completions, connection_manager->allocator); +} - if (aws_linked_list_empty(&connection_manager->pending_acquisitions)) { - if (aws_hash_table_put(&connection_manager->connections, &connection, NULL, NULL)) { - aws_http_connection_close(connection); - } +int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, aws_http_on_client_connection_setup_fn *callback, void *user_data) { - goto done; + struct aws_http_connection_acquisition *request = aws_mem_acquire(connection_manager->allocator, sizeof(struct aws_http_connection_acquisition)); + if (request == NULL) { + callback(NULL, aws_last_error(), user_data); + return AWS_OP_ERR; } - s_aws_http_connection_manager_move_front_acquisition(connection_manager, connection, AWS_ERROR_SUCCESS, &to_complete_acquisitions); + AWS_ZERO_STRUCT(*request); + request->callback = callback; + request->user_data = user_data; - ++connection_manager->vended_connection_count; + struct aws_linked_list completions; + aws_linked_list_init(&completions); -done: + size_t new_connections = 0; + + aws_mutex_lock(&connection_manager->lock); + + AWS_FATAL_ASSERT(connection_manager->state == AWS_HCMST_READY); + + aws_linked_list_push_back(&connection_manager->pending_acquisitions, &request->node); + ++connection_manager->pending_acquisition_count; + + s_aws_http_connection_manager_build_work_order(connection_manager, &completions, &new_connections); aws_mutex_unlock(&connection_manager->lock); - s_aws_http_connection_manager_complete_acquisitions(&to_complete_acquisitions, connection_manager->allocator); + s_aws_http_connection_manager_execute_work_order(connection_manager, &completions, new_connections); - /* - * Delay until the very end just in case someone dropped the last external ref while there's a pending connect - */ - if (return_type == AWS_CAT_NEWLY_CONNECTED) { - aws_http_connection_manager_release(connection_manager); - } + return AWS_OP_SUCCESS; } -/* - * Unlike the other functions, using the to_complete_acquisitions list doesn't make sense here because many - * of the completion points do not have an aws_http_connection_acquisition structure filled out. So just - * do it with local variables (make_callback, callback_connection). - */ -int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, aws_http_on_client_connection_setup_fn *callback, void *user_data) { - int result = AWS_OP_ERR; +int aws_http_connection_manager_release_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection) { + bool should_release = false; - bool make_callback = false; - struct aws_http_connection *callback_connection = NULL; + struct aws_linked_list completions; + aws_linked_list_init(&completions); + + size_t new_connections = 0; aws_mutex_lock(&connection_manager->lock); - /* - * Future: possibly worth evaluating whether LIFO-ordering ready connections gives a performance improvement (cache-wise) - * Would require moving from hash table to a different data structure - */ - size_t available_connection_count = aws_hash_table_get_entry_count(&connection_manager->connections); - if (available_connection_count > 0) { - struct aws_hash_iter iter = aws_hash_iter_begin(&connection_manager->connections); - struct aws_http_connection *connection = (void *) iter.element.key; + AWS_FATAL_ASSERT(connection_manager->state == AWS_HCMST_READY); - aws_hash_iter_delete(&iter, false); + assert(connection_manager->vended_connection_count > 0); + --connection_manager->vended_connection_count; - ++connection_manager->vended_connection_count; - result = AWS_OP_SUCCESS; + should_release = aws_http_connection_is_open(connection); + if (!should_release) { + if (aws_hash_table_put(&connection_manager->connections, &connection, NULL, NULL)) { + should_release = true; + } + } - make_callback = true; - callback_connection = connection; + s_aws_http_connection_manager_build_work_order(connection_manager, &completions, &new_connections); - goto done; - } + aws_mutex_unlock(&connection_manager->lock); - struct aws_http_connection_acquisition *pending_connection = aws_mem_acquire(connection_manager->allocator, sizeof(struct aws_http_connection_acquisition)); - if (pending_connection == NULL) { - make_callback = true; - goto done; - } + s_aws_http_connection_manager_execute_work_order(connection_manager, &completions, new_connections); - if (s_aws_http_connection_manager_new_connection(connection_manager)) { - aws_mem_release(connection_manager->allocator, pending_connection); - make_callback = true; - goto done; + if (should_release) { + connection_manager->functions->release_connection(connection); } - AWS_ZERO_STRUCT(*pending_connection); + return AWS_OP_SUCCESS; +} - pending_connection->callback = callback; - pending_connection->user_data = user_data; +static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_connection *connection, int error_code, void *user_data) { + struct aws_http_connection_manager *manager = user_data; - aws_linked_list_push_back(&connection_manager->pending_acquisitions, &pending_connection->node); - ++connection_manager->pending_acquisition_count; + struct aws_linked_list completions; + aws_linked_list_init(&completions); -done: + size_t new_connections = 0; - aws_mutex_unlock(&connection_manager->lock); + aws_mutex_lock(&manager->lock); - if (make_callback) { - callback(callback_connection, callback_connection != NULL ? AWS_ERROR_SUCCESS : aws_last_error(), user_data); + assert(manager->pending_connects_count > 0); + --manager->pending_connects_count; + + bool is_failure = connection == NULL; + if (connection != NULL) { + if (aws_hash_table_put(&manager->connections, &connection, NULL, NULL)) { + is_failure = true; + } } - return result; + if (is_failure) { + /* + * A nice behavioral optimization. If we failed to connect now, then we're more likely to fail in the near-future + * as well. So if we have an excess of pending acquisitions (beyond the number of pending connects), let's fail + * all of the excess. + */ + while (manager->pending_acquisition_count > manager->pending_connects_count) { + s_aws_http_connection_manager_move_front_acquisition(manager, NULL, error_code, &completions); + } + } + + s_aws_http_connection_manager_build_work_order(manager, &completions, &new_connections); + + aws_mutex_unlock(&manager->lock); + + s_aws_http_connection_manager_execute_work_order(manager, &completions, new_connections); + + if (is_failure && connection) { + manager->functions->release_connection(connection); + } } -int aws_http_connection_manager_release_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection) { - s_aws_http_connection_manager_accept_connection(connection_manager, connection, AWS_CAT_RELEASED); +static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http_connection *connection, int error_code, void *user_data) { + (void)error_code; - return AWS_OP_SUCCESS; + struct aws_http_connection_manager *manager = user_data; + aws_mutex_lock(&manager->lock); + + int was_present = 0; + aws_hash_table_remove(&manager->connections, connection, NULL, &was_present); + + aws_mutex_unlock(&manager->lock); + + if (was_present) { + manager->functions->release_connection(connection); + } } diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c index f2c10dd89..e97056b49 100644 --- a/tests/test_connection_manager.c +++ b/tests/test_connection_manager.c @@ -15,6 +15,7 @@ #include +#include #include #include #include @@ -45,48 +46,13 @@ struct cm_tester { struct aws_tls_ctx_options tls_ctx_options; struct aws_tls_connection_options tls_connection_options; - struct aws_mutex wait_lock; - struct aws_condition_variable wait_cvar; + struct aws_mutex lock; + struct aws_condition_variable signal; - int wait_result; -}; - -#ifdef NEVER -static void s_tester_on_client_connection_setup( - struct aws_http_connection *connection, - int error_code, - void *user_data) { - - struct tester *tester = user_data; - AWS_FATAL_ASSERT(aws_mutex_lock(&tester->wait_lock) == AWS_OP_SUCCESS); - - if (error_code) { - tester->wait_result = error_code; - goto done; - } - - tester->client_connection = connection; - done: - AWS_FATAL_ASSERT(aws_mutex_unlock(&tester->wait_lock) == AWS_OP_SUCCESS); - aws_condition_variable_notify_one(&tester->wait_cvar); -} - -static void s_tester_on_client_connection_shutdown( - struct aws_http_connection *connection, - int error_code, - void *user_data) { - - (void)connection; - (void)error_code; - struct tester *tester = user_data; - AWS_FATAL_ASSERT(aws_mutex_lock(&tester->wait_lock) == AWS_OP_SUCCESS); + struct aws_array_list connections; + size_t connect_errors; - tester->client_connection_is_shutdown = true; - - AWS_FATAL_ASSERT(aws_mutex_unlock(&tester->wait_lock) == AWS_OP_SUCCESS); - aws_condition_variable_notify_one(&tester->wait_cvar); -} -#endif +}; int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options) { AWS_ZERO_STRUCT(*tester); @@ -98,13 +64,7 @@ int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options tester->allocator = options->allocator; - if (aws_mutex_init(&tester->wait_lock)) { - return AWS_OP_ERR; - } - - if (aws_condition_variable_init(&tester->wait_cvar)) { - return AWS_OP_ERR; - } + ASSERT_SUCCESS(aws_array_list_init_dynamic(&tester->connections, tester->allocator, 10, sizeof(struct aws_http_connection *))); ASSERT_SUCCESS(aws_event_loop_group_default_init(&tester->event_loop_group, tester->allocator, 1)); ASSERT_SUCCESS(aws_host_resolver_init_default(&tester->host_resolver, tester->allocator, 8, &tester->event_loop_group)); @@ -142,11 +102,51 @@ int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options return AWS_OP_SUCCESS; } +void s_release_connections(struct cm_tester *tester, size_t count) { + size_t release_count = aws_array_list_length(&tester->connections); + if (release_count > count) { + release_count = count; + } + + for (size_t i = 0; i < release_count; ++i) { + struct aws_http_connection *connection = NULL; + if (aws_array_list_back(&tester->connections, &connection)) { + continue; + } + + aws_array_list_pop_back(&tester->connections); + + aws_http_connection_manager_release_connection(tester->connection_manager, connection); + } +} + +void s_on_acquire_connection(struct aws_http_connection *connection, int error_code, void *user_data) { + struct cm_tester *tester = user_data; + + aws_mutex_lock(&tester->lock); + + if (connection == NULL) { + ++tester->connect_errors; + } else if (aws_array_list_push_back(&tester->connections, &connection)) { + aws_http_connection_manager_release_connection(tester->connection_manager, connection); + } + + aws_mutex_unlock(&tester->lock); +} + +void s_acquire_connections(struct cm_tester *tester, size_t count) { + for (size_t i = 0; i < count; ++i) { + aws_http_connection_manager_acquire_connection(tester->connection_manager, s_on_acquire_connection, tester); + } +} + void s_cm_tester_clean_up(struct cm_tester *tester) { if (tester == NULL) { return; } + s_release_connections(tester, aws_array_list_length(&tester->connections)); + aws_http_connection_manager_release(tester->connection_manager); aws_client_bootstrap_release(tester->client_bootstrap); @@ -158,9 +158,6 @@ void s_cm_tester_clean_up(struct cm_tester *tester) { aws_tls_connection_options_clean_up(&tester->tls_connection_options); aws_tls_ctx_destroy(tester->tls_ctx); - aws_mutex_clean_up(&tester->wait_lock); - aws_condition_variable_clean_up(&tester->wait_cvar); - aws_http_library_clean_up(); aws_tls_clean_up_static_state(); } diff --git a/tests/test_websocket_encoder.c b/tests/test_websocket_encoder.c index 633d8da66..d347d1fa7 100644 --- a/tests/test_websocket_encoder.c +++ b/tests/test_websocket_encoder.c @@ -155,7 +155,7 @@ ENCODER_TEST_CASE(websocket_encoder_rsv) { 0x89, // fin | rsv1 | rsv2 | rsv3 | 4bit opcode 0x00, // mask | 7bit payload len }; - expected_output[0] |= (uint8_t)(1 << (6 - rsv)); + expected_output[0] = (uint8_t)(expected_output[0] | (1 << (6 - rsv))); tester.out_buf.len = 0; /* reset output buffer */ ASSERT_SUCCESS(aws_websocket_encoder_start_frame(&tester.encoder, &input_frame)); From e0bcdc1dd917c1e00dc9c33c0321ce9b9c38a337 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Fri, 10 May 2019 16:05:41 -0700 Subject: [PATCH 04/14] WIP --- include/aws/http/connection_manager.h | 2 +- .../connection_manager_function_table.h | 13 +- source/connection_manager.c | 234 ++++++++++++------ tests/CMakeLists.txt | 1 + tests/test_connection_manager.c | 55 +++- 5 files changed, 223 insertions(+), 82 deletions(-) diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h index 0e5c1b7c6..b3e159cc3 100644 --- a/include/aws/http/connection_manager.h +++ b/include/aws/http/connection_manager.h @@ -41,7 +41,7 @@ struct aws_http_connection_manager_options { struct aws_byte_cursor host; uint16_t port; size_t max_connections; - struct aws_http_connection_manager_function_table *mocks; + const struct aws_http_connection_manager_function_table *mocks; }; AWS_EXTERN_C_BEGIN diff --git a/include/aws/http/private/connection_manager_function_table.h b/include/aws/http/private/connection_manager_function_table.h index 317e23b4e..ffe180ae1 100644 --- a/include/aws/http/private/connection_manager_function_table.h +++ b/include/aws/http/private/connection_manager_function_table.h @@ -23,16 +23,25 @@ typedef int (aws_http_connection_manager_create_connection_fn)(const struct aws_http_client_connection_options *options); typedef void (aws_http_connection_manager_close_connection_fn)(struct aws_http_connection *connection); typedef void (aws_http_connection_manager_release_connection_fn)(struct aws_http_connection *connection); +typedef bool (aws_http_connection_manager_is_connection_open_fn)(const struct aws_http_connection *connection); struct aws_http_connection_manager_function_table { + /* + * Downstream http functions + */ aws_http_connection_manager_create_connection_fn *create_connection; aws_http_connection_manager_close_connection_fn *close_connection; aws_http_connection_manager_release_connection_fn *release_connection; + aws_http_connection_manager_is_connection_open_fn *is_connection_open; + }; AWS_STATIC_IMPL -bool aws_http_connection_manager_function_table_is_valid(struct aws_http_connection_manager_function_table *table) { - return table->create_connection && table->close_connection && table->release_connection; +bool aws_http_connection_manager_function_table_is_valid(const struct aws_http_connection_manager_function_table *table) { + return table->create_connection && table->close_connection && table->release_connection && table->is_connection_open; } +AWS_HTTP_API +extern const struct aws_http_connection_manager_function_table *g_aws_http_connection_manager_default_function_table_ptr; + #endif /* AWS_HTTP_CONNECTION_MANAGER_FUNCTION_TABLE_H */ diff --git a/source/connection_manager.c b/source/connection_manager.c index 1e33e4a03..7a535f5b1 100644 --- a/source/connection_manager.c +++ b/source/connection_manager.c @@ -28,9 +28,12 @@ static struct aws_http_connection_manager_function_table s_default_function_table = { .create_connection = aws_http_client_connect, .release_connection = aws_http_connection_release, - .close_connection = aws_http_connection_close + .close_connection = aws_http_connection_close, + .is_connection_open = aws_http_connection_is_open }; +const struct aws_http_connection_manager_function_table *g_aws_http_connection_manager_default_function_table_ptr = &s_default_function_table; + enum aws_http_connection_manager_state_type { AWS_HCMST_READY, AWS_HCMST_SHUTTING_DOWN @@ -39,19 +42,28 @@ enum aws_http_connection_manager_state_type { struct aws_http_connection_manager { struct aws_allocator *allocator; - struct aws_http_connection_manager_function_table *functions; + /* + * A union of external downstream dependencies (primarily global http API functions) and + * internal implementation references. Selectively overridden by tests in order to + * enable strong coverage of internal implementation details. + */ + const struct aws_http_connection_manager_function_table *functions; /* * Controls access to all mutable state on the connection manager */ struct aws_mutex lock; + /* + * A manager can be in one of two states, READY or SHUTTING_DOWN. The state transition + * takes place when ref_count drops to zero. + */ enum aws_http_connection_manager_state_type state; /* * The set of all available ready-to-be-used connections */ - struct aws_hash_table connections; + struct aws_array_list connections; /* * The set of all incomplete connection acquisition requests @@ -76,6 +88,12 @@ struct aws_http_connection_manager { */ size_t vended_connection_count; + /* + * Always equal to # of connection shutdown callbacks not yet invoked + * or equivalently: + * + * # of connections ever created by the manager - # shutdown callbacks received + */ size_t open_connection_count; /* @@ -84,7 +102,7 @@ struct aws_http_connection_manager { struct aws_client_bootstrap *bootstrap; size_t initial_window_size; struct aws_socket_options socket_options; - struct aws_tls_connection_options tls_connection_options; + struct aws_tls_connection_options *tls_connection_options; struct aws_string *host; uint16_t port; @@ -96,19 +114,32 @@ struct aws_http_connection_manager { /* * Lifecycle tracking for the connection manager. Starts at 1. * - * While state == ready : value = # external refs + # vended connects - * While state == shutting_down : value = # of connection shutdown callbacks not yet invoked + * Once this drops to zero, the state transitions to shutting down + * + * The manager is deleted when all tracking counters have returned to zero. */ size_t ref_count; }; +static bool s_aws_http_connection_manager_should_destroy(struct aws_http_connection_manager *manager) { + if (manager->state != AWS_HCMST_SHUTTING_DOWN) { + return false; + } + + if (manager->vended_connection_count > 0 || manager->pending_connects_count > 0 || manager->open_connection_count > 0) { + return false; + } + + return true; +} + /* * A struct that functions as both the pending acquisition tracker and the about-to-complete data. * * The list in the connection manager is the set of all acquisition requests that we haven't resolved. * * In order to make sure we never invoke callbacks while holding the manager's lock, in a number of places - * we build a list of one or more acquisitions to complete while holding the lock. Once the lock is released + * we build a list of one or more acquisitions to complete. Once the lock is released * we complete all the acquisitions in the list using the data within the struct (hence why we have * connection and result members). */ @@ -121,7 +152,10 @@ struct aws_http_connection_acquisition { }; /* - * Only call this outside the scope of the connection manager's lock + * Invokes a set of acquisition callbacks. Only call this outside the scope of the connection manager's lock. + * + * Assumes that internal state (like pending_acquisition_count, vended_connection_count) have already been updated according + * to the list's contents. */ static void s_aws_http_connection_manager_complete_acquisitions(struct aws_linked_list *acquisitions, struct aws_allocator *allocator) { while (!aws_linked_list_empty(acquisitions)) { @@ -138,7 +172,8 @@ static void s_aws_http_connection_manager_complete_acquisitions(struct aws_linke * build the set of callbacks to be completed once the lock is released. * * If this was a successful acquisition then connection is non-null - * If this was a failed acquisition then connection is null + * If this was a failed acquisition then connection is null and result is hopefully a useful diagnostic (extreme edge cases exist + * where it may not be though) */ static void s_aws_http_connection_manager_move_front_acquisition(struct aws_http_connection_manager *manager, struct aws_http_connection *connection, int result, struct aws_linked_list *output_list) { assert(!aws_linked_list_empty(&manager->pending_acquisitions)); @@ -154,15 +189,6 @@ static void s_aws_http_connection_manager_move_front_acquisition(struct aws_http aws_linked_list_push_back(output_list, node); } -static int s_connection_cleanup(void *context, struct aws_hash_element *element) { - struct aws_http_connection_manager *manager = context; - - struct aws_http_connection *connection = (void *)element->key; - manager->functions->release_connection(connection); - - return AWS_OP_SUCCESS; -} - static void s_aws_http_connection_manager_destroy(struct aws_http_connection_manager *manager) { if (manager == NULL) { return; @@ -173,12 +199,15 @@ static void s_aws_http_connection_manager_destroy(struct aws_http_connection_man assert(manager->pending_acquisition_count == 0); assert(manager->open_connection_count == 0); assert(aws_linked_list_empty(&manager->pending_acquisitions)); - assert(aws_hash_table_get_entry_count(&manager->connections) == 0); + assert(aws_array_list_length(&manager->connections) == 0); - aws_hash_table_clean_up(&manager->connections); + aws_array_list_clean_up(&manager->connections); aws_string_destroy(manager->host); - aws_tls_connection_options_clean_up(&manager->tls_connection_options); + if (manager->tls_connection_options) { + aws_tls_connection_options_clean_up(manager->tls_connection_options); + aws_mem_release(manager->allocator, manager->tls_connection_options); + } aws_mutex_clean_up(&manager->lock); aws_mem_release(manager->allocator, manager); @@ -201,7 +230,7 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_a goto on_error; } - if (aws_hash_table_init(&manager->connections, allocator, 2, aws_hash_ptr, aws_ptr_eq, NULL, NULL)) { + if (aws_array_list_init_dynamic(&manager->connections, allocator, options->max_connections, sizeof(struct aws_http_connection *))) { goto on_error; } @@ -212,8 +241,12 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_a goto on_error; } - if (options->tls_connection_options && aws_tls_connection_options_copy(&manager->tls_connection_options, options->tls_connection_options)) { - goto on_error; + if (options->tls_connection_options) { + manager->tls_connection_options = aws_mem_acquire(allocator, sizeof(struct aws_tls_connection_options)); + AWS_ZERO_STRUCT(*manager->tls_connection_options); + if (aws_tls_connection_options_copy(manager->tls_connection_options, options->tls_connection_options)) { + goto on_error; + } } manager->state = AWS_HCMST_READY; @@ -225,10 +258,12 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_a manager->functions = options->mocks; manager->ref_count = 1; if (manager->functions == NULL) { - manager->functions = &s_default_function_table; + manager->functions = g_aws_http_connection_manager_default_function_table_ptr; } - assert(aws_http_connection_manager_function_table_is_valid(manager->functions)); + if (!aws_http_connection_manager_function_table_is_valid(manager->functions)) { + goto on_error; + } return manager; @@ -245,40 +280,48 @@ void aws_http_connection_manager_acquire(struct aws_http_connection_manager *man aws_mutex_unlock(&manager->lock); } -enum aws_async_release_result { - AWS_ARR_NONE, - AWS_ARR_START_SHUT_DOWN, - AWS_ARR_DELETE -}; +void aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { + struct aws_array_list connections_to_release; + AWS_ZERO_STRUCT(connections_to_release); -static enum aws_async_release_result s_aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { - AWS_FATAL_ASSERT(manager->ref_count > 0); + struct aws_linked_list pending_acquisitions_to_fail; + aws_linked_list_init(&pending_acquisitions_to_fail); - enum aws_async_release_result result = AWS_ARR_NONE; - manager->ref_count -= 1; + bool should_destroy = false; + + aws_mutex_lock(&manager->lock); - if (manager->ref_count == 0 && manager->state == AWS_HCMST_READY) { - assert(manager->vended_connection_count == 0); + AWS_FATAL_ASSERT(manager->ref_count > 0); + manager->ref_count -= 1; + if (manager->ref_count == 0) { manager->state = AWS_HCMST_SHUTTING_DOWN; - manager->ref_count = manager->open_connection_count; - result = manager->ref_count > 0 ? AWS_ARR_START_SHUT_DOWN : AWS_ARR_DELETE; + should_destroy = s_aws_http_connection_manager_should_destroy(manager); + + aws_array_list_init_dynamic(&connections_to_release, manager->allocator, 0, sizeof(struct aws_http_connection *)); + aws_array_list_swap_contents(&manager->connections, &connections_to_release); + aws_linked_list_swap_contents(&manager->pending_acquisitions, &pending_acquisitions_to_fail); } - if (manager->ref_count == 0 && manager->state == AWS_HCMST_SHUTTING_DOWN) { - result = AWS_ARR_DELETE; + aws_mutex_unlock(&manager->lock); + + size_t connection_count = aws_array_list_length(&connections_to_release); + + for (size_t i = 0; i < connection_count; ++i) { + struct aws_http_connection *connection = NULL; + if (aws_array_list_get_at(&connections_to_release, &connection, i)) { + continue; + } + + aws_http_connection_release(connection); } - return result; -} + aws_array_list_clean_up(&connections_to_release); + s_aws_http_connection_manager_complete_acquisitions(&pending_acquisitions_to_fail, manager->allocator); -void aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { - aws_mutex_lock(&manager->lock); - bool start_shutdown_process = s_aws_http_connection_manager_release(manager); - if (start_shutdown_process) { - ??; + if (should_destroy) { + s_aws_http_connection_manager_destroy(manager); } - aws_mutex_unlock(&manager->lock); } static void s_aws_http_connection_manager_build_work_order(struct aws_http_connection_manager *connection_manager, @@ -286,11 +329,11 @@ static void s_aws_http_connection_manager_build_work_order(struct aws_http_conne /* * Step 1 - If there's free connections, complete acquisition requests */ - while(aws_hash_table_get_entry_count(&connection_manager->connections) > 0 && connection_manager->pending_acquisition_count > 0) { - struct aws_hash_iter iter = aws_hash_iter_begin(&connection_manager->connections); - struct aws_http_connection *connection = (void *) iter.element.key; + while(aws_array_list_length(&connection_manager->connections) > 0 && connection_manager->pending_acquisition_count > 0) { + struct aws_http_connection *connection = NULL; + aws_array_list_back(&connection_manager->connections, &connection); - aws_hash_iter_delete(&iter, false); + aws_array_list_pop_back(&connection_manager->connections); s_aws_http_connection_manager_move_front_acquisition(connection_manager, connection, AWS_ERROR_SUCCESS, completions); ++connection_manager->vended_connection_count; @@ -300,9 +343,9 @@ static void s_aws_http_connection_manager_build_work_order(struct aws_http_conne * Step 2 - if there's excess pending acquisitions and we have room to make more, make more */ if (connection_manager->pending_acquisition_count > connection_manager->pending_connects_count) { - *new_connections = connection_manager->pending_acquisition_count - connection_manager->pending_connects_count; - assert(connection_manager->max_connections >= connection_manager->vended_connection_count + connection_manager->pending_connects_count); + + *new_connections = connection_manager->pending_acquisition_count - connection_manager->pending_connects_count; size_t max_new_connections = connection_manager->max_connections - (connection_manager->vended_connection_count + connection_manager->pending_connects_count); if (*new_connections > max_new_connections) { @@ -321,7 +364,7 @@ static int s_aws_http_connection_manager_new_connection(struct aws_http_connecti AWS_ZERO_STRUCT(options); options.self_size = sizeof(struct aws_http_client_connection_options); options.bootstrap = connection_manager->bootstrap; - options.tls_options = &connection_manager->tls_connection_options; + options.tls_options = connection_manager->tls_connection_options; options.allocator = connection_manager->allocator; options.user_data = connection_manager; options.host_name = aws_byte_cursor_from_string(connection_manager->host); @@ -377,6 +420,8 @@ static void s_aws_http_connection_manager_execute_work_order(struct aws_http_con } s_aws_http_connection_manager_complete_acquisitions(completions, connection_manager->allocator); + + aws_array_list_clean_up(&errors); } int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, aws_http_on_client_connection_setup_fn *callback, void *user_data) { @@ -413,8 +458,6 @@ int aws_http_connection_manager_acquire_connection(struct aws_http_connection_ma } int aws_http_connection_manager_release_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection) { - bool should_release = false; - struct aws_linked_list completions; aws_linked_list_init(&completions); @@ -427,10 +470,10 @@ int aws_http_connection_manager_release_connection(struct aws_http_connection_ma assert(connection_manager->vended_connection_count > 0); --connection_manager->vended_connection_count; - should_release = aws_http_connection_is_open(connection); - if (!should_release) { - if (aws_hash_table_put(&connection_manager->connections, &connection, NULL, NULL)) { - should_release = true; + bool should_release_connection = !connection_manager->functions->is_connection_open(connection); + if (!should_release_connection) { + if (aws_array_list_push_back(&connection_manager->connections, &connection)) { + should_release_connection = true; } } @@ -440,7 +483,7 @@ int aws_http_connection_manager_release_connection(struct aws_http_connection_ma s_aws_http_connection_manager_execute_work_order(connection_manager, &completions, new_connections); - if (should_release) { + if (should_release_connection) { connection_manager->functions->release_connection(connection); } @@ -457,21 +500,29 @@ static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_co aws_mutex_lock(&manager->lock); + bool is_shutting_down = manager->state == AWS_HCMST_SHUTTING_DOWN; + assert(manager->pending_acquisition_count == 0 || !is_shutting_down); + assert(manager->pending_connects_count > 0); --manager->pending_connects_count; - bool is_failure = connection == NULL; if (connection != NULL) { - if (aws_hash_table_put(&manager->connections, &connection, NULL, NULL)) { - is_failure = true; + if (!is_shutting_down) { + /* We reserved enough room for max_connections, this should never fail */ + AWS_FATAL_ASSERT(aws_array_list_push_back(&manager->connections, &connection) == AWS_OP_SUCCESS); } + ++manager->open_connection_count; } - if (is_failure) { + bool should_destroy = s_aws_http_connection_manager_should_destroy(manager); + + if (connection == NULL) { /* * A nice behavioral optimization. If we failed to connect now, then we're more likely to fail in the near-future * as well. So if we have an excess of pending acquisitions (beyond the number of pending connects), let's fail - * all of the excess. + * all of the excess rather than wait for a cascade of failures. + * + * This won't happen during shutdown since there are no pending acquisitions at that point. */ while (manager->pending_acquisition_count > manager->pending_connects_count) { s_aws_http_connection_manager_move_front_acquisition(manager, NULL, error_code, &completions); @@ -484,23 +535,62 @@ static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_co s_aws_http_connection_manager_execute_work_order(manager, &completions, new_connections); - if (is_failure && connection) { + if (is_shutting_down && connection != NULL) { + /* + * We didn't add the connection to the pool; just release it immediately + */ manager->functions->release_connection(connection); } + + if (should_destroy) { + s_aws_http_connection_manager_destroy(manager); + } } static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http_connection *connection, int error_code, void *user_data) { (void)error_code; + bool should_release_connection = false; + struct aws_http_connection_manager *manager = user_data; aws_mutex_lock(&manager->lock); - int was_present = 0; - aws_hash_table_remove(&manager->connections, connection, NULL, &was_present); + --manager->open_connection_count; + bool should_destroy = s_aws_http_connection_manager_should_destroy(manager); + + size_t connection_count = aws_array_list_length(&manager->connections); + + if (connection_count > 0) { + assert(manager->state == AWS_HCMST_READY); + + struct aws_http_connection *last_connection = NULL; + aws_array_list_get_at(&manager->connections, &last_connection, connection_count - 1); + + for (size_t i = 0; i < connection_count; ++i) { + struct aws_http_connection *current_connection = NULL; + aws_array_list_get_at(&manager->connections, ¤t_connection, i); + + if (current_connection == connection) { + should_release_connection = true; + aws_array_list_set_at(&manager->connections, &last_connection, i); + break; + } + } + + if (should_release_connection) { + aws_array_list_pop_back(&manager->connections); + } + } aws_mutex_unlock(&manager->lock); - if (was_present) { + assert(!should_release_connection || !should_destroy); + + if (should_release_connection) { manager->functions->release_connection(connection); } + + if (should_destroy) { + s_aws_http_connection_manager_destroy(manager); + } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 105a2e9df..47d56bc81 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -86,6 +86,7 @@ add_test_case(websocket_encoder_fragmentation_failure_checks) add_test_case(websocket_encoder_payload_callback_can_fail_encoder) add_test_case(test_connection_manager_setup_shutdown) +add_test_case(test_connection_manager_single_connection) set(TEST_BINARY_NAME ${CMAKE_PROJECT_NAME}-tests) generate_test_driver(${TEST_BINARY_NAME}) diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c index e97056b49..de922b88d 100644 --- a/tests/test_connection_manager.c +++ b/tests/test_connection_manager.c @@ -52,6 +52,7 @@ struct cm_tester { struct aws_array_list connections; size_t connect_errors; + size_t wait_for_connection_count; }; int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options) { @@ -90,9 +91,9 @@ int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options .bootstrap = tester->client_bootstrap, .initial_window_size = SIZE_MAX, .socket_options = &socket_options, - .tls_connection_options = &tester->tls_connection_options, - .host = aws_byte_cursor_from_c_str("https://s3.amazonaws.com/"), - .port = 443, + .tls_connection_options = NULL, //&tester->tls_connection_options, + .host = aws_byte_cursor_from_c_str("www.google.com"), + .port = 80, .max_connections = options->max_connections }; @@ -127,19 +128,34 @@ void s_on_acquire_connection(struct aws_http_connection *connection, int error_c if (connection == NULL) { ++tester->connect_errors; - } else if (aws_array_list_push_back(&tester->connections, &connection)) { - aws_http_connection_manager_release_connection(tester->connection_manager, connection); + } else { + aws_array_list_push_back(&tester->connections, &connection); } + aws_condition_variable_notify_one(&tester->signal); + aws_mutex_unlock(&tester->lock); } -void s_acquire_connections(struct cm_tester *tester, size_t count) { +static void s_acquire_connections(struct cm_tester *tester, size_t count) { for (size_t i = 0; i < count; ++i) { aws_http_connection_manager_acquire_connection(tester->connection_manager, s_on_acquire_connection, tester); } } +static bool s_is_connection_reply_count_equal(void *context) { + struct cm_tester *tester = context; + + return tester->wait_for_connection_count == aws_array_list_length(&tester->connections) + tester->connect_errors; +} + +static void s_wait_on_connection_reply_count(struct cm_tester *tester, size_t count) { + aws_mutex_lock(&tester->lock); + + tester->wait_for_connection_count = count; + aws_condition_variable_wait_pred(&tester->signal, &tester->lock, s_is_connection_reply_count_equal, tester); +} + void s_cm_tester_clean_up(struct cm_tester *tester) { if (tester == NULL) { return; @@ -147,6 +163,8 @@ void s_cm_tester_clean_up(struct cm_tester *tester) { s_release_connections(tester, aws_array_list_length(&tester->connections)); + aws_array_list_clean_up(&tester->connections); + aws_http_connection_manager_release(tester->connection_manager); aws_client_bootstrap_release(tester->client_bootstrap); @@ -177,4 +195,27 @@ static int s_test_connection_manager_setup_shutdown(struct aws_allocator *alloca return AWS_OP_SUCCESS; } -AWS_TEST_CASE(test_connection_manager_setup_shutdown, s_test_connection_manager_setup_shutdown); \ No newline at end of file +AWS_TEST_CASE(test_connection_manager_setup_shutdown, s_test_connection_manager_setup_shutdown); + +static int s_test_connection_manager_single_connection(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct cm_tester_options options = { + .allocator = allocator, + .max_connections = 5 + }; + + struct cm_tester tester; + ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + + s_acquire_connections(&tester, 1); + + s_wait_on_connection_reply_count(&tester, 1); + + s_release_connections(&tester, 1); + + s_cm_tester_clean_up(&tester); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_connection_manager_single_connection, s_test_connection_manager_single_connection); \ No newline at end of file From c3e4726114a7f588e2391128372690cc8c05cec8 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Mon, 13 May 2019 15:41:13 -0700 Subject: [PATCH 05/14] Simple set of integration tests for connection manager --- include/aws/http/connection_manager.h | 14 +- .../connection_manager_function_table.h | 18 +- source/connection_manager.c | 162 ++++++++++------ tests/CMakeLists.txt | 4 + tests/test_connection_manager.c | 183 +++++++++++++++--- 5 files changed, 278 insertions(+), 103 deletions(-) diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h index b3e159cc3..300e0933d 100644 --- a/include/aws/http/connection_manager.h +++ b/include/aws/http/connection_manager.h @@ -62,21 +62,27 @@ void aws_http_connection_manager_release(struct aws_http_connection_manager *man * Creates a new connection manager with the supplied configuration options. */ AWS_HTTP_API -struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_allocator *allocator, struct aws_http_connection_manager_options *options); +struct aws_http_connection_manager *aws_http_connection_manager_new( + struct aws_allocator *allocator, + struct aws_http_connection_manager_options *options); /* * Requests a connection from the manager */ AWS_HTTP_API -int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, aws_http_on_client_connection_setup_fn *callback, void *user_data); +int aws_http_connection_manager_acquire_connection( + struct aws_http_connection_manager *connection_manager, + aws_http_on_client_connection_setup_fn *callback, + void *user_data); /* * Returns a connection back to the manager */ AWS_HTTP_API -int aws_http_connection_manager_release_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection); +int aws_http_connection_manager_release_connection( + struct aws_http_connection_manager *connection_manager, + struct aws_http_connection *connection); AWS_EXTERN_C_END #endif /* AWS_HTTP_CONNECTION_MANAGER_H */ - diff --git a/include/aws/http/private/connection_manager_function_table.h b/include/aws/http/private/connection_manager_function_table.h index ffe180ae1..03b063745 100644 --- a/include/aws/http/private/connection_manager_function_table.h +++ b/include/aws/http/private/connection_manager_function_table.h @@ -20,10 +20,10 @@ #include -typedef int (aws_http_connection_manager_create_connection_fn)(const struct aws_http_client_connection_options *options); -typedef void (aws_http_connection_manager_close_connection_fn)(struct aws_http_connection *connection); -typedef void (aws_http_connection_manager_release_connection_fn)(struct aws_http_connection *connection); -typedef bool (aws_http_connection_manager_is_connection_open_fn)(const struct aws_http_connection *connection); +typedef int(aws_http_connection_manager_create_connection_fn)(const struct aws_http_client_connection_options *options); +typedef void(aws_http_connection_manager_close_connection_fn)(struct aws_http_connection *connection); +typedef void(aws_http_connection_manager_release_connection_fn)(struct aws_http_connection *connection); +typedef bool(aws_http_connection_manager_is_connection_open_fn)(const struct aws_http_connection *connection); struct aws_http_connection_manager_function_table { /* @@ -33,15 +33,17 @@ struct aws_http_connection_manager_function_table { aws_http_connection_manager_close_connection_fn *close_connection; aws_http_connection_manager_release_connection_fn *release_connection; aws_http_connection_manager_is_connection_open_fn *is_connection_open; - }; AWS_STATIC_IMPL -bool aws_http_connection_manager_function_table_is_valid(const struct aws_http_connection_manager_function_table *table) { - return table->create_connection && table->close_connection && table->release_connection && table->is_connection_open; +bool aws_http_connection_manager_function_table_is_valid( + const struct aws_http_connection_manager_function_table *table) { + return table->create_connection && table->close_connection && table->release_connection && + table->is_connection_open; } AWS_HTTP_API -extern const struct aws_http_connection_manager_function_table *g_aws_http_connection_manager_default_function_table_ptr; +extern const struct aws_http_connection_manager_function_table + *g_aws_http_connection_manager_default_function_table_ptr; #endif /* AWS_HTTP_CONNECTION_MANAGER_FUNCTION_TABLE_H */ diff --git a/source/connection_manager.c b/source/connection_manager.c index 7a535f5b1..a5511dce4 100644 --- a/source/connection_manager.c +++ b/source/connection_manager.c @@ -19,25 +19,22 @@ #include #include #include +#include +#include #include #include #include -#include -#include static struct aws_http_connection_manager_function_table s_default_function_table = { .create_connection = aws_http_client_connect, .release_connection = aws_http_connection_release, .close_connection = aws_http_connection_close, - .is_connection_open = aws_http_connection_is_open -}; + .is_connection_open = aws_http_connection_is_open}; -const struct aws_http_connection_manager_function_table *g_aws_http_connection_manager_default_function_table_ptr = &s_default_function_table; +const struct aws_http_connection_manager_function_table *g_aws_http_connection_manager_default_function_table_ptr = + &s_default_function_table; -enum aws_http_connection_manager_state_type { - AWS_HCMST_READY, - AWS_HCMST_SHUTTING_DOWN -}; +enum aws_http_connection_manager_state_type { AWS_HCMST_READY, AWS_HCMST_SHUTTING_DOWN }; struct aws_http_connection_manager { struct aws_allocator *allocator; @@ -126,7 +123,8 @@ static bool s_aws_http_connection_manager_should_destroy(struct aws_http_connect return false; } - if (manager->vended_connection_count > 0 || manager->pending_connects_count > 0 || manager->open_connection_count > 0) { + if (manager->vended_connection_count > 0 || manager->pending_connects_count > 0 || + manager->open_connection_count > 0) { return false; } @@ -154,15 +152,19 @@ struct aws_http_connection_acquisition { /* * Invokes a set of acquisition callbacks. Only call this outside the scope of the connection manager's lock. * - * Assumes that internal state (like pending_acquisition_count, vended_connection_count) have already been updated according - * to the list's contents. + * Assumes that internal state (like pending_acquisition_count, vended_connection_count) have already been updated + * according to the list's contents. */ -static void s_aws_http_connection_manager_complete_acquisitions(struct aws_linked_list *acquisitions, struct aws_allocator *allocator) { +static void s_aws_http_connection_manager_complete_acquisitions( + struct aws_linked_list *acquisitions, + struct aws_allocator *allocator) { while (!aws_linked_list_empty(acquisitions)) { struct aws_linked_list_node *node = aws_linked_list_pop_front(acquisitions); - struct aws_http_connection_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_http_connection_acquisition, node); + struct aws_http_connection_acquisition *pending_acquisition = + AWS_CONTAINER_OF(node, struct aws_http_connection_acquisition, node); - pending_acquisition->callback(pending_acquisition->connection, pending_acquisition->result, pending_acquisition->user_data); + pending_acquisition->callback( + pending_acquisition->connection, pending_acquisition->result, pending_acquisition->user_data); aws_mem_release(allocator, pending_acquisition); } } @@ -172,17 +174,22 @@ static void s_aws_http_connection_manager_complete_acquisitions(struct aws_linke * build the set of callbacks to be completed once the lock is released. * * If this was a successful acquisition then connection is non-null - * If this was a failed acquisition then connection is null and result is hopefully a useful diagnostic (extreme edge cases exist - * where it may not be though) + * If this was a failed acquisition then connection is null and result is hopefully a useful diagnostic (extreme edge + * cases exist where it may not be though) */ -static void s_aws_http_connection_manager_move_front_acquisition(struct aws_http_connection_manager *manager, struct aws_http_connection *connection, int result, struct aws_linked_list *output_list) { - assert(!aws_linked_list_empty(&manager->pending_acquisitions)); - assert(manager->pending_acquisition_count > 0); +static void s_aws_http_connection_manager_move_front_acquisition( + struct aws_http_connection_manager *manager, + struct aws_http_connection *connection, + int result, + struct aws_linked_list *output_list) { + AWS_ASSERT(!aws_linked_list_empty(&manager->pending_acquisitions)); + AWS_ASSERT(manager->pending_acquisition_count > 0); struct aws_linked_list_node *node = aws_linked_list_pop_front(&manager->pending_acquisitions); --manager->pending_acquisition_count; - struct aws_http_connection_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_http_connection_acquisition, node); + struct aws_http_connection_acquisition *pending_acquisition = + AWS_CONTAINER_OF(node, struct aws_http_connection_acquisition, node); pending_acquisition->connection = connection; pending_acquisition->result = result; @@ -194,12 +201,12 @@ static void s_aws_http_connection_manager_destroy(struct aws_http_connection_man return; } - assert(manager->pending_connects_count == 0); - assert(manager->vended_connection_count == 0); - assert(manager->pending_acquisition_count == 0); - assert(manager->open_connection_count == 0); - assert(aws_linked_list_empty(&manager->pending_acquisitions)); - assert(aws_array_list_length(&manager->connections) == 0); + AWS_ASSERT(manager->pending_connects_count == 0); + AWS_ASSERT(manager->vended_connection_count == 0); + AWS_ASSERT(manager->pending_acquisition_count == 0); + AWS_ASSERT(manager->open_connection_count == 0); + AWS_ASSERT(aws_linked_list_empty(&manager->pending_acquisitions)); + AWS_ASSERT(aws_array_list_length(&manager->connections) == 0); aws_array_list_clean_up(&manager->connections); @@ -213,12 +220,15 @@ static void s_aws_http_connection_manager_destroy(struct aws_http_connection_man aws_mem_release(manager->allocator, manager); } -struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_allocator *allocator, struct aws_http_connection_manager_options *options) { - assert(options); - assert(options->socket_options); - assert(options->max_connections > 0); +struct aws_http_connection_manager *aws_http_connection_manager_new( + struct aws_allocator *allocator, + struct aws_http_connection_manager_options *options) { + AWS_ASSERT(options); + AWS_ASSERT(options->socket_options); + AWS_ASSERT(options->max_connections > 0); - struct aws_http_connection_manager *manager = aws_mem_acquire(allocator, sizeof(struct aws_http_connection_manager)); + struct aws_http_connection_manager *manager = + aws_mem_acquire(allocator, sizeof(struct aws_http_connection_manager)); if (manager == NULL) { return NULL; } @@ -230,7 +240,8 @@ struct aws_http_connection_manager *aws_http_connection_manager_new(struct aws_a goto on_error; } - if (aws_array_list_init_dynamic(&manager->connections, allocator, options->max_connections, sizeof(struct aws_http_connection *))) { + if (aws_array_list_init_dynamic( + &manager->connections, allocator, options->max_connections, sizeof(struct aws_http_connection *))) { goto on_error; } @@ -298,7 +309,8 @@ void aws_http_connection_manager_release(struct aws_http_connection_manager *man manager->state = AWS_HCMST_SHUTTING_DOWN; should_destroy = s_aws_http_connection_manager_should_destroy(manager); - aws_array_list_init_dynamic(&connections_to_release, manager->allocator, 0, sizeof(struct aws_http_connection *)); + aws_array_list_init_dynamic( + &connections_to_release, manager->allocator, 0, sizeof(struct aws_http_connection *)); aws_array_list_swap_contents(&manager->connections, &connections_to_release); aws_linked_list_swap_contents(&manager->pending_acquisitions, &pending_acquisitions_to_fail); } @@ -324,18 +336,22 @@ void aws_http_connection_manager_release(struct aws_http_connection_manager *man } } -static void s_aws_http_connection_manager_build_work_order(struct aws_http_connection_manager *connection_manager, - struct aws_linked_list *completions, size_t *new_connections) { +static void s_aws_http_connection_manager_build_work_order( + struct aws_http_connection_manager *connection_manager, + struct aws_linked_list *completions, + size_t *new_connections) { /* * Step 1 - If there's free connections, complete acquisition requests */ - while(aws_array_list_length(&connection_manager->connections) > 0 && connection_manager->pending_acquisition_count > 0) { + while (aws_array_list_length(&connection_manager->connections) > 0 && + connection_manager->pending_acquisition_count > 0) { struct aws_http_connection *connection = NULL; aws_array_list_back(&connection_manager->connections, &connection); aws_array_list_pop_back(&connection_manager->connections); - s_aws_http_connection_manager_move_front_acquisition(connection_manager, connection, AWS_ERROR_SUCCESS, completions); + s_aws_http_connection_manager_move_front_acquisition( + connection_manager, connection, AWS_ERROR_SUCCESS, completions); ++connection_manager->vended_connection_count; } @@ -343,10 +359,14 @@ static void s_aws_http_connection_manager_build_work_order(struct aws_http_conne * Step 2 - if there's excess pending acquisitions and we have room to make more, make more */ if (connection_manager->pending_acquisition_count > connection_manager->pending_connects_count) { - assert(connection_manager->max_connections >= connection_manager->vended_connection_count + connection_manager->pending_connects_count); + AWS_ASSERT( + connection_manager->max_connections >= + connection_manager->vended_connection_count + connection_manager->pending_connects_count); *new_connections = connection_manager->pending_acquisition_count - connection_manager->pending_connects_count; - size_t max_new_connections = connection_manager->max_connections - (connection_manager->vended_connection_count + connection_manager->pending_connects_count); + size_t max_new_connections = + connection_manager->max_connections - + (connection_manager->vended_connection_count + connection_manager->pending_connects_count); if (*new_connections > max_new_connections) { *new_connections = max_new_connections; @@ -356,8 +376,14 @@ static void s_aws_http_connection_manager_build_work_order(struct aws_http_conne } } -static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_connection *connection, int error_code, void *user_data); -static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http_connection *connection, int error_code, void *user_data); +static void s_aws_http_connection_manager_on_connection_setup( + struct aws_http_connection *connection, + int error_code, + void *user_data); +static void s_aws_http_connection_manager_on_connection_shutdown( + struct aws_http_connection *connection, + int error_code, + void *user_data); static int s_aws_http_connection_manager_new_connection(struct aws_http_connection_manager *connection_manager) { struct aws_http_client_connection_options options; @@ -374,18 +400,17 @@ static int s_aws_http_connection_manager_new_connection(struct aws_http_connecti options.on_setup = s_aws_http_connection_manager_on_connection_setup; options.on_shutdown = s_aws_http_connection_manager_on_connection_shutdown; - if (connection_manager->functions->create_connection(&options)) - { + if (connection_manager->functions->create_connection(&options)) { return AWS_OP_ERR; } return AWS_OP_SUCCESS; } - -static void s_aws_http_connection_manager_execute_work_order(struct aws_http_connection_manager *connection_manager, - struct aws_linked_list *completions, - size_t new_connections) { +static void s_aws_http_connection_manager_execute_work_order( + struct aws_http_connection_manager *connection_manager, + struct aws_linked_list *completions, + size_t new_connections) { int representative_error = 0; size_t new_connection_failures = 0; @@ -407,7 +432,8 @@ static void s_aws_http_connection_manager_execute_work_order(struct aws_http_con connection_manager->pending_connects_count -= new_connection_failures; for (size_t i = 0; - i < new_connection_failures && !aws_linked_list_empty(&connection_manager->pending_acquisitions); ++i) { + i < new_connection_failures && !aws_linked_list_empty(&connection_manager->pending_acquisitions); + ++i) { int error = representative_error; if (i < aws_array_list_length(&errors)) { aws_array_list_get_at(&errors, &error, i); @@ -424,9 +450,13 @@ static void s_aws_http_connection_manager_execute_work_order(struct aws_http_con aws_array_list_clean_up(&errors); } -int aws_http_connection_manager_acquire_connection(struct aws_http_connection_manager *connection_manager, aws_http_on_client_connection_setup_fn *callback, void *user_data) { +int aws_http_connection_manager_acquire_connection( + struct aws_http_connection_manager *connection_manager, + aws_http_on_client_connection_setup_fn *callback, + void *user_data) { - struct aws_http_connection_acquisition *request = aws_mem_acquire(connection_manager->allocator, sizeof(struct aws_http_connection_acquisition)); + struct aws_http_connection_acquisition *request = + aws_mem_acquire(connection_manager->allocator, sizeof(struct aws_http_connection_acquisition)); if (request == NULL) { callback(NULL, aws_last_error(), user_data); return AWS_OP_ERR; @@ -457,7 +487,9 @@ int aws_http_connection_manager_acquire_connection(struct aws_http_connection_ma return AWS_OP_SUCCESS; } -int aws_http_connection_manager_release_connection(struct aws_http_connection_manager *connection_manager, struct aws_http_connection *connection) { +int aws_http_connection_manager_release_connection( + struct aws_http_connection_manager *connection_manager, + struct aws_http_connection *connection) { struct aws_linked_list completions; aws_linked_list_init(&completions); @@ -467,7 +499,7 @@ int aws_http_connection_manager_release_connection(struct aws_http_connection_ma AWS_FATAL_ASSERT(connection_manager->state == AWS_HCMST_READY); - assert(connection_manager->vended_connection_count > 0); + AWS_ASSERT(connection_manager->vended_connection_count > 0); --connection_manager->vended_connection_count; bool should_release_connection = !connection_manager->functions->is_connection_open(connection); @@ -490,7 +522,10 @@ int aws_http_connection_manager_release_connection(struct aws_http_connection_ma return AWS_OP_SUCCESS; } -static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_connection *connection, int error_code, void *user_data) { +static void s_aws_http_connection_manager_on_connection_setup( + struct aws_http_connection *connection, + int error_code, + void *user_data) { struct aws_http_connection_manager *manager = user_data; struct aws_linked_list completions; @@ -501,9 +536,9 @@ static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_co aws_mutex_lock(&manager->lock); bool is_shutting_down = manager->state == AWS_HCMST_SHUTTING_DOWN; - assert(manager->pending_acquisition_count == 0 || !is_shutting_down); + AWS_ASSERT(manager->pending_acquisition_count == 0 || !is_shutting_down); - assert(manager->pending_connects_count > 0); + AWS_ASSERT(manager->pending_connects_count > 0); --manager->pending_connects_count; if (connection != NULL) { @@ -518,9 +553,9 @@ static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_co if (connection == NULL) { /* - * A nice behavioral optimization. If we failed to connect now, then we're more likely to fail in the near-future - * as well. So if we have an excess of pending acquisitions (beyond the number of pending connects), let's fail - * all of the excess rather than wait for a cascade of failures. + * A nice behavioral optimization. If we failed to connect now, then we're more likely to fail in the + * near-future as well. So if we have an excess of pending acquisitions (beyond the number of pending + * connects), let's fail all of the excess rather than wait for a cascade of failures. * * This won't happen during shutdown since there are no pending acquisitions at that point. */ @@ -547,7 +582,10 @@ static void s_aws_http_connection_manager_on_connection_setup(struct aws_http_co } } -static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http_connection *connection, int error_code, void *user_data) { +static void s_aws_http_connection_manager_on_connection_shutdown( + struct aws_http_connection *connection, + int error_code, + void *user_data) { (void)error_code; bool should_release_connection = false; @@ -561,7 +599,7 @@ static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http size_t connection_count = aws_array_list_length(&manager->connections); if (connection_count > 0) { - assert(manager->state == AWS_HCMST_READY); + AWS_ASSERT(manager->state == AWS_HCMST_READY); struct aws_http_connection *last_connection = NULL; aws_array_list_get_at(&manager->connections, &last_connection, connection_count - 1); @@ -584,7 +622,7 @@ static void s_aws_http_connection_manager_on_connection_shutdown(struct aws_http aws_mutex_unlock(&manager->lock); - assert(!should_release_connection || !should_destroy); + AWS_ASSERT(!should_release_connection || !should_destroy); if (should_release_connection) { manager->functions->release_connection(connection); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 47d56bc81..3735da182 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -87,6 +87,10 @@ add_test_case(websocket_encoder_payload_callback_can_fail_encoder) add_test_case(test_connection_manager_setup_shutdown) add_test_case(test_connection_manager_single_connection) +add_test_case(test_connection_manager_many_connections) +add_test_case(test_connection_manager_acquire_release) +add_test_case(test_connection_manager_close_and_release) +add_test_case(test_connection_manager_acquire_release_mix) set(TEST_BINARY_NAME ${CMAKE_PROJECT_NAME}-tests) generate_test_driver(${TEST_BINARY_NAME}) diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c index de922b88d..6a2ba3df0 100644 --- a/tests/test_connection_manager.c +++ b/tests/test_connection_manager.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -50,7 +51,8 @@ struct cm_tester { struct aws_condition_variable signal; struct aws_array_list connections; - size_t connect_errors; + size_t connection_errors; + size_t connection_releases; size_t wait_for_connection_count; }; @@ -65,19 +67,20 @@ int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options tester->allocator = options->allocator; - ASSERT_SUCCESS(aws_array_list_init_dynamic(&tester->connections, tester->allocator, 10, sizeof(struct aws_http_connection *))); + ASSERT_SUCCESS( + aws_array_list_init_dynamic(&tester->connections, tester->allocator, 10, sizeof(struct aws_http_connection *))); ASSERT_SUCCESS(aws_event_loop_group_default_init(&tester->event_loop_group, tester->allocator, 1)); - ASSERT_SUCCESS(aws_host_resolver_init_default(&tester->host_resolver, tester->allocator, 8, &tester->event_loop_group)); + ASSERT_SUCCESS( + aws_host_resolver_init_default(&tester->host_resolver, tester->allocator, 8, &tester->event_loop_group)); tester->client_bootstrap = - aws_client_bootstrap_new(tester->allocator, &tester->event_loop_group, &tester->host_resolver, NULL); + aws_client_bootstrap_new(tester->allocator, &tester->event_loop_group, &tester->host_resolver, NULL); ASSERT_NOT_NULL(tester->client_bootstrap); struct aws_socket_options socket_options = { .type = AWS_SOCKET_STREAM, .domain = AWS_SOCKET_IPV4, - .connect_timeout_ms = - (uint32_t)aws_timestamp_convert(60, AWS_TIMESTAMP_SECS, AWS_TIMESTAMP_MILLIS, NULL), + .connect_timeout_ms = (uint32_t)aws_timestamp_convert(60, AWS_TIMESTAMP_SECS, AWS_TIMESTAMP_MILLIS, NULL), }; aws_tls_ctx_options_init_default_client(&tester->tls_ctx_options, options->allocator); @@ -87,15 +90,14 @@ int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options aws_tls_connection_options_init_from_ctx(&tester->tls_connection_options, tester->tls_ctx); - struct aws_http_connection_manager_options cm_options = { - .bootstrap = tester->client_bootstrap, - .initial_window_size = SIZE_MAX, - .socket_options = &socket_options, - .tls_connection_options = NULL, //&tester->tls_connection_options, - .host = aws_byte_cursor_from_c_str("www.google.com"), - .port = 80, - .max_connections = options->max_connections - }; + struct aws_http_connection_manager_options cm_options = {.bootstrap = tester->client_bootstrap, + .initial_window_size = SIZE_MAX, + .socket_options = &socket_options, + .tls_connection_options = + NULL, //&tester->tls_connection_options, + .host = aws_byte_cursor_from_c_str("www.google.com"), + .port = 80, + .max_connections = options->max_connections}; tester->connection_manager = aws_http_connection_manager_new(tester->allocator, &cm_options); ASSERT_NOT_NULL(tester->connection_manager); @@ -103,12 +105,18 @@ int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options return AWS_OP_SUCCESS; } -void s_release_connections(struct cm_tester *tester, size_t count) { +void s_release_connections(struct cm_tester *tester, size_t count, bool close_first) { + + aws_mutex_lock(&tester->lock); + size_t release_count = aws_array_list_length(&tester->connections); if (release_count > count) { release_count = count; } + struct aws_array_list to_release; + aws_array_list_init_dynamic(&to_release, tester->allocator, release_count, sizeof(struct aws_http_connection *)); + for (size_t i = 0; i < release_count; ++i) { struct aws_http_connection *connection = NULL; if (aws_array_list_back(&tester->connections, &connection)) { @@ -117,8 +125,30 @@ void s_release_connections(struct cm_tester *tester, size_t count) { aws_array_list_pop_back(&tester->connections); + aws_array_list_push_back(&to_release, &connection); + } + + aws_mutex_unlock(&tester->lock); + + for (size_t i = 0; i < aws_array_list_length(&to_release); ++i) { + struct aws_http_connection *connection = NULL; + if (aws_array_list_get_at(&to_release, &connection, i)) { + continue; + } + + if (close_first) { + aws_http_connection_close(connection); + } + aws_http_connection_manager_release_connection(tester->connection_manager, connection); + + aws_mutex_lock(&tester->lock); + ++tester->connection_releases; + aws_condition_variable_notify_one(&tester->signal); + aws_mutex_unlock(&tester->lock); } + + aws_array_list_clean_up(&to_release); } void s_on_acquire_connection(struct aws_http_connection *connection, int error_code, void *user_data) { @@ -127,7 +157,7 @@ void s_on_acquire_connection(struct aws_http_connection *connection, int error_c aws_mutex_lock(&tester->lock); if (connection == NULL) { - ++tester->connect_errors; + ++tester->connection_errors; } else { aws_array_list_push_back(&tester->connections, &connection); } @@ -146,7 +176,8 @@ static void s_acquire_connections(struct cm_tester *tester, size_t count) { static bool s_is_connection_reply_count_equal(void *context) { struct cm_tester *tester = context; - return tester->wait_for_connection_count == aws_array_list_length(&tester->connections) + tester->connect_errors; + return tester->wait_for_connection_count == + aws_array_list_length(&tester->connections) + tester->connection_errors + tester->connection_releases; } static void s_wait_on_connection_reply_count(struct cm_tester *tester, size_t count) { @@ -154,6 +185,8 @@ static void s_wait_on_connection_reply_count(struct cm_tester *tester, size_t co tester->wait_for_connection_count = count; aws_condition_variable_wait_pred(&tester->signal, &tester->lock, s_is_connection_reply_count_equal, tester); + + aws_mutex_unlock(&tester->lock); } void s_cm_tester_clean_up(struct cm_tester *tester) { @@ -161,7 +194,7 @@ void s_cm_tester_clean_up(struct cm_tester *tester) { return; } - s_release_connections(tester, aws_array_list_length(&tester->connections)); + s_release_connections(tester, aws_array_list_length(&tester->connections), false); aws_array_list_clean_up(&tester->connections); @@ -183,10 +216,7 @@ void s_cm_tester_clean_up(struct cm_tester *tester) { static int s_test_connection_manager_setup_shutdown(struct aws_allocator *allocator, void *ctx) { (void)ctx; - struct cm_tester_options options = { - .allocator = allocator, - .max_connections = 5 - }; + struct cm_tester_options options = {.allocator = allocator, .max_connections = 5}; struct cm_tester tester; ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); @@ -200,10 +230,7 @@ AWS_TEST_CASE(test_connection_manager_setup_shutdown, s_test_connection_manager_ static int s_test_connection_manager_single_connection(struct aws_allocator *allocator, void *ctx) { (void)ctx; - struct cm_tester_options options = { - .allocator = allocator, - .max_connections = 5 - }; + struct cm_tester_options options = {.allocator = allocator, .max_connections = 5}; struct cm_tester tester; ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); @@ -212,10 +239,108 @@ static int s_test_connection_manager_single_connection(struct aws_allocator *all s_wait_on_connection_reply_count(&tester, 1); - s_release_connections(&tester, 1); + s_release_connections(&tester, 1, false); + + s_cm_tester_clean_up(&tester); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_connection_manager_single_connection, s_test_connection_manager_single_connection); + +static int s_test_connection_manager_many_connections(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct cm_tester_options options = {.allocator = allocator, .max_connections = 20}; + + struct cm_tester tester; + ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + + s_acquire_connections(&tester, 20); + + s_wait_on_connection_reply_count(&tester, 20); + + s_release_connections(&tester, 20, false); + + s_cm_tester_clean_up(&tester); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_connection_manager_many_connections, s_test_connection_manager_many_connections); + +static int s_test_connection_manager_acquire_release(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct cm_tester_options options = {.allocator = allocator, .max_connections = 4}; + + struct cm_tester tester; + ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + + s_acquire_connections(&tester, 20); + + s_wait_on_connection_reply_count(&tester, 4); + + for (size_t i = 4; i < 20; ++i) { + s_release_connections(&tester, 1, false); + + s_wait_on_connection_reply_count(&tester, i + 1); + } + + s_cm_tester_clean_up(&tester); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_connection_manager_acquire_release, s_test_connection_manager_acquire_release); + +static int s_test_connection_manager_close_and_release(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct cm_tester_options options = {.allocator = allocator, .max_connections = 4}; + + struct cm_tester tester; + ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + + s_acquire_connections(&tester, 20); + + s_wait_on_connection_reply_count(&tester, 4); + + for (size_t i = 4; i < 20; ++i) { + s_release_connections(&tester, 1, i % 1 == 0); + + s_wait_on_connection_reply_count(&tester, i + 1); + } + + s_cm_tester_clean_up(&tester); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_connection_manager_close_and_release, s_test_connection_manager_close_and_release); + +static int s_test_connection_manager_acquire_release_mix(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct cm_tester_options options = {.allocator = allocator, .max_connections = 5}; + + struct cm_tester tester; + ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + + for (size_t i = 0; i < 10; ++i) { + s_acquire_connections(&tester, 2); + + s_wait_on_connection_reply_count(&tester, i + 1); + + s_release_connections(&tester, 1, i % 1 == 0); + } + + s_wait_on_connection_reply_count(&tester, 15); + + for (size_t i = 15; i < 20; ++i) { + s_release_connections(&tester, 1, i % 1 == 0); + + s_wait_on_connection_reply_count(&tester, i + 1); + } s_cm_tester_clean_up(&tester); return AWS_OP_SUCCESS; } -AWS_TEST_CASE(test_connection_manager_single_connection, s_test_connection_manager_single_connection); \ No newline at end of file +AWS_TEST_CASE(test_connection_manager_acquire_release_mix, s_test_connection_manager_acquire_release_mix); \ No newline at end of file From d563af9d7e28f5be15e9b8625a0c14bde4fd61a8 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Tue, 14 May 2019 16:09:07 -0700 Subject: [PATCH 06/14] More tests and fixes --- include/aws/http/connection_manager.h | 16 +- source/connection_manager.c | 111 +++++---- tests/CMakeLists.txt | 4 + tests/test_connection_manager.c | 330 +++++++++++++++++++++----- 4 files changed, 363 insertions(+), 98 deletions(-) diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h index 300e0933d..eaccde22b 100644 --- a/include/aws/http/connection_manager.h +++ b/include/aws/http/connection_manager.h @@ -34,14 +34,25 @@ struct aws_http_connection_manager_mocks; * the maximum number of connections to ever have in existence. */ struct aws_http_connection_manager_options { + /* + * http connection configuration + */ struct aws_client_bootstrap *bootstrap; size_t initial_window_size; struct aws_socket_options *socket_options; struct aws_tls_connection_options *tls_connection_options; struct aws_byte_cursor host; uint16_t port; + + /* + * Maximum number of connections this manager is allowed to contain + */ size_t max_connections; - const struct aws_http_connection_manager_function_table *mocks; + + /* + * Private function table for mocking http connection management + */ + const struct aws_http_connection_manager_function_table *function_table; }; AWS_EXTERN_C_BEGIN @@ -67,7 +78,8 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( struct aws_http_connection_manager_options *options); /* - * Requests a connection from the manager + * Requests a connection from the manager. The requester is notified of + * an acquired connection (or failure to acquire) via the supplied callback. */ AWS_HTTP_API int aws_http_connection_manager_acquire_connection( diff --git a/source/connection_manager.c b/source/connection_manager.c index a5511dce4..bfa1fa677 100644 --- a/source/connection_manager.c +++ b/source/connection_manager.c @@ -25,6 +25,9 @@ #include #include +/* + * Function table to use under normal circumstances + */ static struct aws_http_connection_manager_function_table s_default_function_table = { .create_connection = aws_http_client_connect, .release_connection = aws_http_connection_release, @@ -44,7 +47,7 @@ struct aws_http_connection_manager { * internal implementation references. Selectively overridden by tests in order to * enable strong coverage of internal implementation details. */ - const struct aws_http_connection_manager_function_table *functions; + const struct aws_http_connection_manager_function_table *function_table; /* * Controls access to all mutable state on the connection manager @@ -58,7 +61,7 @@ struct aws_http_connection_manager { enum aws_http_connection_manager_state_type state; /* - * The set of all available ready-to-be-used connections + * The set of all available, ready-to-be-used connections */ struct aws_array_list connections; @@ -75,8 +78,7 @@ struct aws_http_connection_manager { /* * The number of pending new connection requests we have outstanding to the http - * layer. Each pending new connection requests adds one to the connection manager's - * overall ref count. Each resolved request subtracts one. + * layer. */ size_t pending_connects_count; @@ -111,18 +113,28 @@ struct aws_http_connection_manager { /* * Lifecycle tracking for the connection manager. Starts at 1. * - * Once this drops to zero, the state transitions to shutting down + * Once this drops to zero, the manager state transitions to shutting down * - * The manager is deleted when all tracking counters have returned to zero. + * The manager is deleted when all other tracking counters have returned to zero. + * + * We don't use an atomic here because the shutdown phase wants to check many different + * values. You could argue that we could use a sum of everything, but we still need the + * individual values for proper behavior and error checking during the ready state. Also, + * a hybrid atomic/lock solution felt excessively complicated and delicate. */ - size_t ref_count; + size_t external_ref_count; }; +/* + * The manager's lock must be held by the caller. + */ static bool s_aws_http_connection_manager_should_destroy(struct aws_http_connection_manager *manager) { if (manager->state != AWS_HCMST_SHUTTING_DOWN) { return false; } + AWS_ASSERT(manager->external_ref_count == 0); + if (manager->vended_connection_count > 0 || manager->pending_connects_count > 0 || manager->open_connection_count > 0) { return false; @@ -146,25 +158,27 @@ struct aws_http_connection_acquisition { aws_http_on_client_connection_setup_fn *callback; void *user_data; struct aws_http_connection *connection; - int result; + int error_code; }; /* - * Invokes a set of acquisition callbacks. Only call this outside the scope of the connection manager's lock. + * Invokes a set of acquisition completion callbacks. Only call this outside the scope of the connection manager's + * lock. * - * Assumes that internal state (like pending_acquisition_count, vended_connection_count) have already been updated - * according to the list's contents. + * Assumes that internal state (like pending_acquisition_count, vended_connection_count, etc...) have already been + * updated according to the list's contents. */ static void s_aws_http_connection_manager_complete_acquisitions( struct aws_linked_list *acquisitions, struct aws_allocator *allocator) { + while (!aws_linked_list_empty(acquisitions)) { struct aws_linked_list_node *node = aws_linked_list_pop_front(acquisitions); struct aws_http_connection_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_http_connection_acquisition, node); pending_acquisition->callback( - pending_acquisition->connection, pending_acquisition->result, pending_acquisition->user_data); + pending_acquisition->connection, pending_acquisition->error_code, pending_acquisition->user_data); aws_mem_release(allocator, pending_acquisition); } } @@ -174,14 +188,15 @@ static void s_aws_http_connection_manager_complete_acquisitions( * build the set of callbacks to be completed once the lock is released. * * If this was a successful acquisition then connection is non-null - * If this was a failed acquisition then connection is null and result is hopefully a useful diagnostic (extreme edge - * cases exist where it may not be though) + * If this was a failed acquisition then connection is null and error_code is hopefully a useful diagnostic (extreme + * edge cases exist where it may not be though) */ static void s_aws_http_connection_manager_move_front_acquisition( struct aws_http_connection_manager *manager, struct aws_http_connection *connection, - int result, + int error_code, struct aws_linked_list *output_list) { + AWS_ASSERT(!aws_linked_list_empty(&manager->pending_acquisitions)); AWS_ASSERT(manager->pending_acquisition_count > 0); @@ -191,7 +206,7 @@ static void s_aws_http_connection_manager_move_front_acquisition( struct aws_http_connection_acquisition *pending_acquisition = AWS_CONTAINER_OF(node, struct aws_http_connection_acquisition, node); pending_acquisition->connection = connection; - pending_acquisition->result = result; + pending_acquisition->error_code = error_code; aws_linked_list_push_back(output_list, node); } @@ -215,6 +230,7 @@ static void s_aws_http_connection_manager_destroy(struct aws_http_connection_man aws_tls_connection_options_clean_up(manager->tls_connection_options); aws_mem_release(manager->allocator, manager->tls_connection_options); } + aws_mutex_clean_up(&manager->lock); aws_mem_release(manager->allocator, manager); @@ -223,6 +239,7 @@ static void s_aws_http_connection_manager_destroy(struct aws_http_connection_man struct aws_http_connection_manager *aws_http_connection_manager_new( struct aws_allocator *allocator, struct aws_http_connection_manager_options *options) { + AWS_ASSERT(options); AWS_ASSERT(options->socket_options); AWS_ASSERT(options->max_connections > 0); @@ -266,13 +283,13 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( manager->max_connections = options->max_connections; manager->socket_options = *options->socket_options; manager->bootstrap = options->bootstrap; - manager->functions = options->mocks; - manager->ref_count = 1; - if (manager->functions == NULL) { - manager->functions = g_aws_http_connection_manager_default_function_table_ptr; + manager->function_table = options->function_table; + manager->external_ref_count = 1; + if (manager->function_table == NULL) { + manager->function_table = g_aws_http_connection_manager_default_function_table_ptr; } - if (!aws_http_connection_manager_function_table_is_valid(manager->functions)) { + if (!aws_http_connection_manager_function_table_is_valid(manager->function_table)) { goto on_error; } @@ -287,7 +304,7 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( void aws_http_connection_manager_acquire(struct aws_http_connection_manager *manager) { aws_mutex_lock(&manager->lock); - manager->ref_count += 1; + manager->external_ref_count += 1; aws_mutex_unlock(&manager->lock); } @@ -302,10 +319,10 @@ void aws_http_connection_manager_release(struct aws_http_connection_manager *man aws_mutex_lock(&manager->lock); - AWS_FATAL_ASSERT(manager->ref_count > 0); - manager->ref_count -= 1; + AWS_FATAL_ASSERT(manager->external_ref_count > 0); + manager->external_ref_count -= 1; - if (manager->ref_count == 0) { + if (manager->external_ref_count == 0) { manager->state = AWS_HCMST_SHUTTING_DOWN; should_destroy = s_aws_http_connection_manager_should_destroy(manager); @@ -325,7 +342,7 @@ void aws_http_connection_manager_release(struct aws_http_connection_manager *man continue; } - aws_http_connection_release(connection); + manager->function_table->release_connection(connection); } aws_array_list_clean_up(&connections_to_release); @@ -400,7 +417,7 @@ static int s_aws_http_connection_manager_new_connection(struct aws_http_connecti options.on_setup = s_aws_http_connection_manager_on_connection_setup; options.on_shutdown = s_aws_http_connection_manager_on_connection_shutdown; - if (connection_manager->functions->create_connection(&options)) { + if (connection_manager->function_table->create_connection(&options)) { return AWS_OP_ERR; } @@ -418,10 +435,13 @@ static void s_aws_http_connection_manager_execute_work_order( struct aws_array_list errors; AWS_ZERO_STRUCT(errors); - if (!aws_array_list_init_dynamic(&errors, connection_manager->allocator, new_connections, sizeof(int))) { - for (size_t i = 0; i < new_connections; ++i) { - if (s_aws_http_connection_manager_new_connection(connection_manager)) { - representative_error = aws_last_error(); + bool push_errors = aws_array_list_init_dynamic( + &errors, connection_manager->allocator, new_connections, sizeof(int)) == AWS_ERROR_SUCCESS; + for (size_t i = 0; i < new_connections; ++i) { + if (s_aws_http_connection_manager_new_connection(connection_manager)) { + ++new_connection_failures; + representative_error = aws_last_error(); + if (push_errors) { aws_array_list_push_back(&errors, &representative_error); } } @@ -431,15 +451,24 @@ static void s_aws_http_connection_manager_execute_work_order( aws_mutex_lock(&connection_manager->lock); connection_manager->pending_connects_count -= new_connection_failures; - for (size_t i = 0; - i < new_connection_failures && !aws_linked_list_empty(&connection_manager->pending_acquisitions); - ++i) { + size_t i = 0; + + /* + * Rather than failing one acquisition for each connection failure, if there's at least one + * connection failure, we instead fail all excess acquisitions, since there's no pending + * connect that will necessarily resolve them. + * + * Try to correspond an error with the acquisition failure, but as a fallback just use the + * representative error + */ + while (connection_manager->pending_acquisition_count > connection_manager->pending_connects_count) { int error = representative_error; if (i < aws_array_list_length(&errors)) { aws_array_list_get_at(&errors, &error, i); } s_aws_http_connection_manager_move_front_acquisition(connection_manager, NULL, error, completions); + ++i; } aws_mutex_unlock(&connection_manager->lock); @@ -502,7 +531,7 @@ int aws_http_connection_manager_release_connection( AWS_ASSERT(connection_manager->vended_connection_count > 0); --connection_manager->vended_connection_count; - bool should_release_connection = !connection_manager->functions->is_connection_open(connection); + bool should_release_connection = !connection_manager->function_table->is_connection_open(connection); if (!should_release_connection) { if (aws_array_list_push_back(&connection_manager->connections, &connection)) { should_release_connection = true; @@ -516,7 +545,7 @@ int aws_http_connection_manager_release_connection( s_aws_http_connection_manager_execute_work_order(connection_manager, &completions, new_connections); if (should_release_connection) { - connection_manager->functions->release_connection(connection); + connection_manager->function_table->release_connection(connection); } return AWS_OP_SUCCESS; @@ -553,9 +582,9 @@ static void s_aws_http_connection_manager_on_connection_setup( if (connection == NULL) { /* - * A nice behavioral optimization. If we failed to connect now, then we're more likely to fail in the - * near-future as well. So if we have an excess of pending acquisitions (beyond the number of pending - * connects), let's fail all of the excess rather than wait for a cascade of failures. + * To be safe, if we have an excess of pending acquisitions (beyond the number of pending + * connects), we need to fail all of the excess. Technically, we might be able to try and + * make a new connection, if there's room, but that could lead to some bad failure loops. * * This won't happen during shutdown since there are no pending acquisitions at that point. */ @@ -574,7 +603,7 @@ static void s_aws_http_connection_manager_on_connection_setup( /* * We didn't add the connection to the pool; just release it immediately */ - manager->functions->release_connection(connection); + manager->function_table->release_connection(connection); } if (should_destroy) { @@ -625,7 +654,7 @@ static void s_aws_http_connection_manager_on_connection_shutdown( AWS_ASSERT(!should_release_connection || !should_destroy); if (should_release_connection) { - manager->functions->release_connection(connection); + manager->function_table->release_connection(connection); } if (should_destroy) { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 3735da182..9d1c7de2d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -91,6 +91,10 @@ add_test_case(test_connection_manager_many_connections) add_test_case(test_connection_manager_acquire_release) add_test_case(test_connection_manager_close_and_release) add_test_case(test_connection_manager_acquire_release_mix) +add_test_case(test_connection_manager_acquire_release_mix_synchronous) +add_test_case(test_connection_manager_connect_callback_failure) +add_test_case(test_connection_manager_connect_immediate_failure) +add_test_case(test_connection_manager_success_then_cancel_pending_from_failure) set(TEST_BINARY_NAME ${CMAKE_PROJECT_NAME}-tests) generate_test_driver(${TEST_BINARY_NAME}) diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c index 6a2ba3df0..33deeaef3 100644 --- a/tests/test_connection_manager.c +++ b/tests/test_connection_manager.c @@ -23,14 +23,23 @@ #include #include #include +#include #include #include #include #include #include +enum new_connection_result_type { AWS_NCRT_SUCCESS, AWS_NCRT_ERROR_VIA_CALLBACK, AWS_NCRT_ERROR_FROM_CREATE }; + +struct mock_connection_proxy { + enum new_connection_result_type result; + bool is_closed_on_release; +}; + struct cm_tester_options { struct aws_allocator *allocator; + struct aws_http_connection_manager_function_table *mock_table; size_t max_connections; }; @@ -55,9 +64,19 @@ struct cm_tester { size_t connection_releases; size_t wait_for_connection_count; + + struct aws_http_connection_manager_function_table *mock_table; + + struct aws_atomic_var next_connection_id; + struct aws_array_list mock_connections; + struct aws_atomic_var release_connection_fn; }; -int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options) { +static struct cm_tester s_tester; + +int s_cm_tester_init(struct cm_tester_options *options) { + struct cm_tester *tester = &s_tester; + AWS_ZERO_STRUCT(*tester); aws_tls_init_static_state(options->allocator); @@ -97,15 +116,40 @@ int s_cm_tester_init(struct cm_tester *tester, struct cm_tester_options *options NULL, //&tester->tls_connection_options, .host = aws_byte_cursor_from_c_str("www.google.com"), .port = 80, - .max_connections = options->max_connections}; + .max_connections = options->max_connections, + .function_table = options->mock_table}; tester->connection_manager = aws_http_connection_manager_new(tester->allocator, &cm_options); ASSERT_NOT_NULL(tester->connection_manager); + tester->mock_table = options->mock_table; + + aws_atomic_store_int(&tester->next_connection_id, 0); + aws_atomic_store_ptr(&tester->release_connection_fn, NULL); + + ASSERT_SUCCESS(aws_array_list_init_dynamic( + &tester->mock_connections, tester->allocator, 10, sizeof(struct mock_connection_proxy *))); + return AWS_OP_SUCCESS; } -void s_release_connections(struct cm_tester *tester, size_t count, bool close_first) { +void s_add_mock_connections(size_t count, enum new_connection_result_type result, bool closed_on_release) { + struct cm_tester *tester = &s_tester; + + for (size_t i = 0; i < count; ++i) { + struct mock_connection_proxy *mock = aws_mem_acquire(tester->allocator, sizeof(struct mock_connection_proxy)); + AWS_ZERO_STRUCT(*mock); + + mock->result = result; + mock->is_closed_on_release = closed_on_release; + + aws_array_list_push_back(&tester->mock_connections, &mock); + } +} + +void s_release_connections(size_t count, bool close_first) { + + struct cm_tester *tester = &s_tester; aws_mutex_lock(&tester->lock); @@ -137,7 +181,11 @@ void s_release_connections(struct cm_tester *tester, size_t count, bool close_fi } if (close_first) { - aws_http_connection_close(connection); + if (tester->mock_table) { + tester->mock_table->close_connection(connection); + } else { + aws_http_connection_close(connection); + } } aws_http_connection_manager_release_connection(tester->connection_manager, connection); @@ -152,7 +200,7 @@ void s_release_connections(struct cm_tester *tester, size_t count, bool close_fi } void s_on_acquire_connection(struct aws_http_connection *connection, int error_code, void *user_data) { - struct cm_tester *tester = user_data; + struct cm_tester *tester = &s_tester; aws_mutex_lock(&tester->lock); @@ -167,37 +215,50 @@ void s_on_acquire_connection(struct aws_http_connection *connection, int error_c aws_mutex_unlock(&tester->lock); } -static void s_acquire_connections(struct cm_tester *tester, size_t count) { +static void s_acquire_connections(size_t count) { + struct cm_tester *tester = &s_tester; + for (size_t i = 0; i < count; ++i) { aws_http_connection_manager_acquire_connection(tester->connection_manager, s_on_acquire_connection, tester); } } -static bool s_is_connection_reply_count_equal(void *context) { - struct cm_tester *tester = context; +static bool s_is_connection_reply_count_at_least(void *context) { + struct cm_tester *tester = &s_tester; - return tester->wait_for_connection_count == + return tester->wait_for_connection_count <= aws_array_list_length(&tester->connections) + tester->connection_errors + tester->connection_releases; } -static void s_wait_on_connection_reply_count(struct cm_tester *tester, size_t count) { +static void s_wait_on_connection_reply_count(size_t count) { + struct cm_tester *tester = &s_tester; + aws_mutex_lock(&tester->lock); tester->wait_for_connection_count = count; - aws_condition_variable_wait_pred(&tester->signal, &tester->lock, s_is_connection_reply_count_equal, tester); + aws_condition_variable_wait_pred(&tester->signal, &tester->lock, s_is_connection_reply_count_at_least, tester); aws_mutex_unlock(&tester->lock); } -void s_cm_tester_clean_up(struct cm_tester *tester) { - if (tester == NULL) { - return; - } +void s_cm_tester_clean_up(void) { + struct cm_tester *tester = &s_tester; - s_release_connections(tester, aws_array_list_length(&tester->connections), false); + s_release_connections(aws_array_list_length(&tester->connections), false); aws_array_list_clean_up(&tester->connections); + for (size_t i = 0; i < aws_array_list_length(&tester->mock_connections); ++i) { + struct mock_connection_proxy *mock = NULL; + + if (aws_array_list_get_at(&tester->mock_connections, &mock, i)) { + continue; + } + + aws_mem_release(tester->allocator, mock); + } + aws_array_list_clean_up(&tester->mock_connections); + aws_http_connection_manager_release(tester->connection_manager); aws_client_bootstrap_release(tester->client_bootstrap); @@ -218,10 +279,9 @@ static int s_test_connection_manager_setup_shutdown(struct aws_allocator *alloca struct cm_tester_options options = {.allocator = allocator, .max_connections = 5}; - struct cm_tester tester; - ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + ASSERT_SUCCESS(s_cm_tester_init(&options)); - s_cm_tester_clean_up(&tester); + s_cm_tester_clean_up(); return AWS_OP_SUCCESS; } @@ -232,16 +292,15 @@ static int s_test_connection_manager_single_connection(struct aws_allocator *all struct cm_tester_options options = {.allocator = allocator, .max_connections = 5}; - struct cm_tester tester; - ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + ASSERT_SUCCESS(s_cm_tester_init(&options)); - s_acquire_connections(&tester, 1); + s_acquire_connections(1); - s_wait_on_connection_reply_count(&tester, 1); + s_wait_on_connection_reply_count(1); - s_release_connections(&tester, 1, false); + s_release_connections(1, false); - s_cm_tester_clean_up(&tester); + s_cm_tester_clean_up(); return AWS_OP_SUCCESS; } @@ -252,16 +311,15 @@ static int s_test_connection_manager_many_connections(struct aws_allocator *allo struct cm_tester_options options = {.allocator = allocator, .max_connections = 20}; - struct cm_tester tester; - ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + ASSERT_SUCCESS(s_cm_tester_init(&options)); - s_acquire_connections(&tester, 20); + s_acquire_connections(20); - s_wait_on_connection_reply_count(&tester, 20); + s_wait_on_connection_reply_count(20); - s_release_connections(&tester, 20, false); + s_release_connections(20, false); - s_cm_tester_clean_up(&tester); + s_cm_tester_clean_up(); return AWS_OP_SUCCESS; } @@ -272,20 +330,19 @@ static int s_test_connection_manager_acquire_release(struct aws_allocator *alloc struct cm_tester_options options = {.allocator = allocator, .max_connections = 4}; - struct cm_tester tester; - ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + ASSERT_SUCCESS(s_cm_tester_init(&options)); - s_acquire_connections(&tester, 20); + s_acquire_connections(20); - s_wait_on_connection_reply_count(&tester, 4); + s_wait_on_connection_reply_count(4); for (size_t i = 4; i < 20; ++i) { - s_release_connections(&tester, 1, false); + s_release_connections(1, false); - s_wait_on_connection_reply_count(&tester, i + 1); + s_wait_on_connection_reply_count(i + 1); } - s_cm_tester_clean_up(&tester); + s_cm_tester_clean_up(); return AWS_OP_SUCCESS; } @@ -296,20 +353,19 @@ static int s_test_connection_manager_close_and_release(struct aws_allocator *all struct cm_tester_options options = {.allocator = allocator, .max_connections = 4}; - struct cm_tester tester; - ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + ASSERT_SUCCESS(s_cm_tester_init(&options)); - s_acquire_connections(&tester, 20); + s_acquire_connections(20); - s_wait_on_connection_reply_count(&tester, 4); + s_wait_on_connection_reply_count(4); for (size_t i = 4; i < 20; ++i) { - s_release_connections(&tester, 1, i % 1 == 0); + s_release_connections(1, i % 1 == 0); - s_wait_on_connection_reply_count(&tester, i + 1); + s_wait_on_connection_reply_count(i + 1); } - s_cm_tester_clean_up(&tester); + s_cm_tester_clean_up(); return AWS_OP_SUCCESS; } @@ -320,27 +376,191 @@ static int s_test_connection_manager_acquire_release_mix(struct aws_allocator *a struct cm_tester_options options = {.allocator = allocator, .max_connections = 5}; - struct cm_tester tester; - ASSERT_SUCCESS(s_cm_tester_init(&tester, &options)); + ASSERT_SUCCESS(s_cm_tester_init(&options)); + + for (size_t i = 0; i < 10; ++i) { + s_acquire_connections(2); + + s_wait_on_connection_reply_count(i + 1); + + s_release_connections(1, i % 1 == 0); + } + + s_wait_on_connection_reply_count(15); + + for (size_t i = 15; i < 20; ++i) { + s_release_connections(1, i % 1 == 0); + + s_wait_on_connection_reply_count(i + 1); + } + + s_cm_tester_clean_up(); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_connection_manager_acquire_release_mix, s_test_connection_manager_acquire_release_mix); + +static int s_aws_http_connection_manager_create_connection_sync_mock( + const struct aws_http_client_connection_options *options) { + struct cm_tester *tester = &s_tester; + + size_t next_connection_id = aws_atomic_fetch_add(&tester->next_connection_id, 1); + aws_atomic_store_ptr(&tester->release_connection_fn, options->on_shutdown); + + struct mock_connection_proxy *connection = NULL; + + if (next_connection_id < aws_array_list_length(&tester->mock_connections)) { + aws_array_list_get_at(&tester->mock_connections, &connection, next_connection_id); + } + + if (connection->result == AWS_NCRT_SUCCESS) { + options->on_setup((struct aws_http_connection *)connection, AWS_ERROR_SUCCESS, options->user_data); + } else if (connection->result == AWS_NCRT_ERROR_VIA_CALLBACK) { + options->on_setup(NULL, AWS_ERROR_HTTP_UNKNOWN, options->user_data); + } + + return connection->result != AWS_NCRT_ERROR_FROM_CREATE ? AWS_ERROR_SUCCESS : AWS_ERROR_HTTP_UNKNOWN; +} + +static void s_aws_http_connection_manager_release_connection_sync_mock(struct aws_http_connection *connection) { + (void)connection; + + struct cm_tester *tester = &s_tester; + + aws_http_on_client_connection_shutdown_fn *release_callback = aws_atomic_load_ptr(&tester->release_connection_fn); + + release_callback(connection, AWS_ERROR_SUCCESS, tester->connection_manager); +} + +static void s_aws_http_connection_manager_close_connection_sync_mock(struct aws_http_connection *connection) { + (void)connection; +} + +static bool s_aws_http_connection_manager_is_connection_open_sync_mock(const struct aws_http_connection *connection) { + (void)connection; + + struct mock_connection_proxy *proxy = (struct mock_connection_proxy *)(void *)connection; + + return !proxy->is_closed_on_release; +} + +static struct aws_http_connection_manager_function_table s_synchronous_mocks = { + .create_connection = s_aws_http_connection_manager_create_connection_sync_mock, + .release_connection = s_aws_http_connection_manager_release_connection_sync_mock, + .close_connection = s_aws_http_connection_manager_close_connection_sync_mock, + .is_connection_open = s_aws_http_connection_manager_is_connection_open_sync_mock}; + +static int s_test_connection_manager_acquire_release_mix_synchronous(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct cm_tester_options options = { + .allocator = allocator, .max_connections = 5, .mock_table = &s_synchronous_mocks}; + + ASSERT_SUCCESS(s_cm_tester_init(&options)); + + for (size_t i = 0; i < 20; ++i) { + s_add_mock_connections(1, AWS_NCRT_SUCCESS, i % 1 == 0); + } for (size_t i = 0; i < 10; ++i) { - s_acquire_connections(&tester, 2); + s_acquire_connections(2); - s_wait_on_connection_reply_count(&tester, i + 1); + s_wait_on_connection_reply_count(i + 1); - s_release_connections(&tester, 1, i % 1 == 0); + s_release_connections(1, false); } - s_wait_on_connection_reply_count(&tester, 15); + s_wait_on_connection_reply_count(15); for (size_t i = 15; i < 20; ++i) { - s_release_connections(&tester, 1, i % 1 == 0); + s_release_connections(1, false); - s_wait_on_connection_reply_count(&tester, i + 1); + s_wait_on_connection_reply_count(i + 1); } - s_cm_tester_clean_up(&tester); + ASSERT_TRUE(s_tester.connection_errors == 0); + + s_cm_tester_clean_up(); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE( + test_connection_manager_acquire_release_mix_synchronous, + s_test_connection_manager_acquire_release_mix_synchronous); + +static int s_test_connection_manager_connect_callback_failure(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct cm_tester_options options = { + .allocator = allocator, .max_connections = 5, .mock_table = &s_synchronous_mocks}; + + ASSERT_SUCCESS(s_cm_tester_init(&options)); + + s_add_mock_connections(5, AWS_NCRT_ERROR_VIA_CALLBACK, false); + + s_acquire_connections(5); + + s_wait_on_connection_reply_count(5); + + ASSERT_TRUE(s_tester.connection_errors == 5); + + s_cm_tester_clean_up(); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_connection_manager_connect_callback_failure, s_test_connection_manager_connect_callback_failure); + +static int s_test_connection_manager_connect_immediate_failure(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + + struct cm_tester_options options = { + .allocator = allocator, .max_connections = 5, .mock_table = &s_synchronous_mocks}; + + ASSERT_SUCCESS(s_cm_tester_init(&options)); + + s_add_mock_connections(5, AWS_NCRT_ERROR_FROM_CREATE, false); + + s_acquire_connections(5); + + s_wait_on_connection_reply_count(5); + + ASSERT_TRUE(s_tester.connection_errors == 5); + + s_cm_tester_clean_up(); + + return AWS_OP_SUCCESS; +} +AWS_TEST_CASE(test_connection_manager_connect_immediate_failure, s_test_connection_manager_connect_immediate_failure); + +static int s_test_connection_manager_success_then_cancel_pending_from_failure( + struct aws_allocator *allocator, + void *ctx) { + (void)ctx; + + struct cm_tester_options options = { + .allocator = allocator, .max_connections = 1, .mock_table = &s_synchronous_mocks}; + + ASSERT_SUCCESS(s_cm_tester_init(&options)); + + s_add_mock_connections(1, AWS_NCRT_SUCCESS, true); + s_add_mock_connections(1, AWS_NCRT_ERROR_FROM_CREATE, false); + + s_acquire_connections(5); + + s_wait_on_connection_reply_count(1); + + ASSERT_TRUE(s_tester.connection_errors == 0); + + s_release_connections(1, true); + + s_wait_on_connection_reply_count(5); + + ASSERT_TRUE(s_tester.connection_errors == 4); + + s_cm_tester_clean_up(); return AWS_OP_SUCCESS; } -AWS_TEST_CASE(test_connection_manager_acquire_release_mix, s_test_connection_manager_acquire_release_mix); \ No newline at end of file +AWS_TEST_CASE( + test_connection_manager_success_then_cancel_pending_from_failure, + s_test_connection_manager_success_then_cancel_pending_from_failure); \ No newline at end of file From 0301c63af63350cc272316eada2abfce1f1ba89a Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Wed, 15 May 2019 13:30:29 -0700 Subject: [PATCH 07/14] Logging, invariants, bug fixes --- include/aws/http/connection_manager.h | 4 +- include/aws/http/http.h | 3 + source/connection_manager.c | 411 +++++++++++++++++++------- source/http.c | 8 +- 4 files changed, 324 insertions(+), 102 deletions(-) diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h index eaccde22b..2f8498465 100644 --- a/include/aws/http/connection_manager.h +++ b/include/aws/http/connection_manager.h @@ -83,7 +83,7 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( */ AWS_HTTP_API int aws_http_connection_manager_acquire_connection( - struct aws_http_connection_manager *connection_manager, + struct aws_http_connection_manager *manager, aws_http_on_client_connection_setup_fn *callback, void *user_data); @@ -92,7 +92,7 @@ int aws_http_connection_manager_acquire_connection( */ AWS_HTTP_API int aws_http_connection_manager_release_connection( - struct aws_http_connection_manager *connection_manager, + struct aws_http_connection_manager *manager, struct aws_http_connection *connection); AWS_EXTERN_C_END diff --git a/include/aws/http/http.h b/include/aws/http/http.h index c6d3ca9d5..8b8150f0b 100644 --- a/include/aws/http/http.h +++ b/include/aws/http/http.h @@ -28,6 +28,8 @@ enum aws_http_errors { AWS_ERROR_HTTP_REACTION_REQUIRED, AWS_ERROR_HTTP_DATA_NOT_AVAILABLE, AWS_ERROR_HTTP_OUTGOING_STREAM_LENGTH_INCORRECT, + AWS_ERROR_HTTP_CONNECTION_MANAGER_INVALID_STATE_FOR_ACQUIRE, + AWS_ERROR_HTTP_CONNECTION_MANAGER_VENDED_CONNECTION_UNDERFLOW, AWS_ERROR_HTTP_END_RANGE = 0x0C00, }; @@ -36,6 +38,7 @@ enum aws_http_log_subject { AWS_LS_HTTP_CONNECTION, AWS_LS_HTTP_SERVER, AWS_LS_HTTP_STREAM, + AWS_LS_HTTP_CONNECTION_MANAGER, }; enum aws_http_version { diff --git a/source/connection_manager.c b/source/connection_manager.c index bfa1fa677..685e9af7e 100644 --- a/source/connection_manager.c +++ b/source/connection_manager.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -39,6 +40,66 @@ const struct aws_http_connection_manager_function_table *g_aws_http_connection_m enum aws_http_connection_manager_state_type { AWS_HCMST_READY, AWS_HCMST_SHUTTING_DOWN }; +/** + * Vocabulary + * Acquisition - a request by a user for a connection + * Pending Acquisition - a request by a user for a new connection that has not been completed. It may be + * waiting on http, a release by another user, or the manager itself. + * Pending Connect - a request to the http layer for a new connection that has not been resolved yet + * Vended Connection - a successfully established connection that is currently in use by something; must + * be released (through the connection manager) by the user before anyone else can use it. The connection + * manager does not explicitly track vended connections. + * Task Set - A set of operations that should be attempted once the lock is released. A task set includes + * completion callbacks (which can't fail) and connection attempts (which can fail either immediately or + * asynchronously). + * + * Requirements/Assumptions + * (1) Don't invoke user callbacks while holding the internal state lock + * (2) Don't invoke downstream http calls while holding the internal state lock + * (3) Only log unusual or rare events while the lock is held. Common-path logging should be while it is + * not held. + * (4) Don't crash or do awful things (leaking resources is ok though) if the interface contract + * (ref counting + balanced acquire/release of connections) is violated by the user + * + * In order to fulfill (1) and (2), all operations within the connection manager follow a pattern: + * + * (1) Lock + * (2) Make state changes based on the operation + * (3) Build a task set (completions and connect calls) as appropriate to the operation + * (4) Unlock + * (5) Execute the task set + * + * Asynchronous work order failures are handled in the async callback, but immediate failures require + * us to relock and update the internal state. When there's an immediate connect failure, we use a + * conservative policy to fail all excess (beyond the # of pending connects) acquisitions; this allows us + * to avoid a possible recursive invocation (and potential failures) to connect again. + * + * Lifecycle + * Our connection manager implementation has a reasonably complex lifecycle. + * + * All state around the life cycle is protected by a lock. It seemed too risky and error-prone + * to try and mix an atomic ref count with the internal tracking counters we need. + * + * Over the course of its lifetime, a connection manager moves through two states: + * + * READY - connections may be acquired and released. When the external ref count for the manager + * drops to zero, the manager moves to: + * + * SHUTTING_DOWN - connections may no longer be acquired and released (how could they if the external + * ref count was accurate?) but in case of user ref errors, we simply fail attempts to do so rather + * than crash or underflow. While in this state, we wait for a set of tracking counters to all fall to zero: + * + * pending_connect_count - the # of unresolved calls to the http layer's connect logic + * open_connection_count - the # of connections for whom the release callback (from http) has not been invoked + * vended_connection_count - the # of connections held by external users that haven't been released. Under correct + * usage this should be zero before SHUTTING_DOWN is entered, but we attempt to handle incorrect usage gracefully. + * + * While shutting down, as pending connects resolve, we immediately release new incoming (from http) connections + * + * During the transition from READY to SHUTTING_DOWN, we flush the pending acquisition queue (with failure callbacks) + * and since we disallow new acquires, pending_acquisition_count should always be zero after the transition. + * + */ struct aws_http_connection_manager { struct aws_allocator *allocator; @@ -126,14 +187,20 @@ struct aws_http_connection_manager { }; /* - * The manager's lock must be held by the caller. + * Hard Requirement: Manager's lock must held somewhere in the call stack */ static bool s_aws_http_connection_manager_should_destroy(struct aws_http_connection_manager *manager) { if (manager->state != AWS_HCMST_SHUTTING_DOWN) { return false; } - AWS_ASSERT(manager->external_ref_count == 0); + if (manager->external_ref_count != 0) { + AWS_LOGF_ERROR( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: ref count is non zero while in the shut down state", + (void *)manager); + return false; + } if (manager->vended_connection_count > 0 || manager->pending_connects_count > 0 || manager->open_connection_count > 0) { @@ -146,15 +213,18 @@ static bool s_aws_http_connection_manager_should_destroy(struct aws_http_connect /* * A struct that functions as both the pending acquisition tracker and the about-to-complete data. * - * The list in the connection manager is the set of all acquisition requests that we haven't resolved. + * The list in the connection manager (pending_acquisitions) is the set of all acquisition requests that we + * haven't yet resolved. * * In order to make sure we never invoke callbacks while holding the manager's lock, in a number of places * we build a list of one or more acquisitions to complete. Once the lock is released * we complete all the acquisitions in the list using the data within the struct (hence why we have - * connection and result members). + * "result-oriented" members like connection and error_code). This means we can fail an acquisition + * simply by setting the error_code and moving it to the current task set. */ struct aws_http_connection_acquisition { struct aws_linked_list_node node; + struct aws_http_connection_manager *manager; /* Only used by logging */ aws_http_on_client_connection_setup_fn *callback; void *user_data; struct aws_http_connection *connection; @@ -162,8 +232,9 @@ struct aws_http_connection_acquisition { }; /* - * Invokes a set of acquisition completion callbacks. Only call this outside the scope of the connection manager's - * lock. + * Invokes a set of connection acquisition completion callbacks. + * + * Soft Requirement: The manager's lock must not be held in the callstack. * * Assumes that internal state (like pending_acquisition_count, vended_connection_count, etc...) have already been * updated according to the list's contents. @@ -179,14 +250,32 @@ static void s_aws_http_connection_manager_complete_acquisitions( pending_acquisition->callback( pending_acquisition->connection, pending_acquisition->error_code, pending_acquisition->user_data); + + if (pending_acquisition->error_code != 0) { + AWS_LOGF_WARN( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: Failed to completed connection acquisition with error_code %d(%s)", + (void *)pending_acquisition->manager, + pending_acquisition->error_code, + aws_error_str(pending_acquisition->error_code)); + } else { + AWS_LOGF_DEBUG( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: Successfully completed connection acquisition with connection id=%p", + (void *)pending_acquisition->manager, + (void *)pending_acquisition->connection); + } + aws_mem_release(allocator, pending_acquisition); } } /* - * Moves the first pending connection acquisition into a list. Call this while holding the lock to + * Moves the first pending connection acquisition into a (task set) list. Call this while holding the lock to * build the set of callbacks to be completed once the lock is released. * + * Hard Requirement: Manager's lock must held somewhere in the call stack + * * If this was a successful acquisition then connection is non-null * If this was a failed acquisition then connection is null and error_code is hopefully a useful diagnostic (extreme * edge cases exist where it may not be though) @@ -197,10 +286,10 @@ static void s_aws_http_connection_manager_move_front_acquisition( int error_code, struct aws_linked_list *output_list) { - AWS_ASSERT(!aws_linked_list_empty(&manager->pending_acquisitions)); - AWS_ASSERT(manager->pending_acquisition_count > 0); - + AWS_FATAL_ASSERT(!aws_linked_list_empty(&manager->pending_acquisitions)); struct aws_linked_list_node *node = aws_linked_list_pop_front(&manager->pending_acquisitions); + + AWS_FATAL_ASSERT(manager->pending_acquisition_count > 0); --manager->pending_acquisition_count; struct aws_http_connection_acquisition *pending_acquisition = @@ -216,6 +305,8 @@ static void s_aws_http_connection_manager_destroy(struct aws_http_connection_man return; } + AWS_LOGF_INFO(AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: Destroying self", (void *)manager); + AWS_ASSERT(manager->pending_connects_count == 0); AWS_ASSERT(manager->vended_connection_count == 0); AWS_ASSERT(manager->pending_acquisition_count == 0); @@ -240,9 +331,10 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( struct aws_allocator *allocator, struct aws_http_connection_manager_options *options) { - AWS_ASSERT(options); - AWS_ASSERT(options->socket_options); - AWS_ASSERT(options->max_connections > 0); + if (!options || !options->socket_options || options->max_connections == 0) { + aws_raise_error(AWS_ERROR_INVALID_ARGUMENT); + return NULL; + } struct aws_http_connection_manager *manager = aws_mem_acquire(allocator, sizeof(struct aws_http_connection_manager)); @@ -293,6 +385,8 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( goto on_error; } + AWS_LOGF_INFO(AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: Successfully created", (void *)manager); + return manager; on_error: @@ -309,6 +403,9 @@ void aws_http_connection_manager_acquire(struct aws_http_connection_manager *man } void aws_http_connection_manager_release(struct aws_http_connection_manager *manager) { + /* + * Swap targets in case we need to start the shut down process (clean up done outside the lock) + */ struct aws_array_list connections_to_release; AWS_ZERO_STRUCT(connections_to_release); @@ -317,24 +414,57 @@ void aws_http_connection_manager_release(struct aws_http_connection_manager *man bool should_destroy = false; - aws_mutex_lock(&manager->lock); + AWS_LOGF_INFO(AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: release", (void *)manager); - AWS_FATAL_ASSERT(manager->external_ref_count > 0); - manager->external_ref_count -= 1; - - if (manager->external_ref_count == 0) { - manager->state = AWS_HCMST_SHUTTING_DOWN; - should_destroy = s_aws_http_connection_manager_should_destroy(manager); + aws_mutex_lock(&manager->lock); - aws_array_list_init_dynamic( - &connections_to_release, manager->allocator, 0, sizeof(struct aws_http_connection *)); - aws_array_list_swap_contents(&manager->connections, &connections_to_release); - aws_linked_list_swap_contents(&manager->pending_acquisitions, &pending_acquisitions_to_fail); + if (manager->external_ref_count > 0) { + manager->external_ref_count -= 1; + + if (manager->external_ref_count == 0) { + AWS_LOGF_INFO( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: ref count now zero, starting shut down process", + (void *)manager); + manager->state = AWS_HCMST_SHUTTING_DOWN; + should_destroy = s_aws_http_connection_manager_should_destroy(manager); + + /* + * swap our internal connection set with the zeroed local set + */ + aws_array_list_init_dynamic( + &connections_to_release, manager->allocator, 0, sizeof(struct aws_http_connection *)); + aws_array_list_swap_contents(&manager->connections, &connections_to_release); + + /* + * Swap our pending acquisitions with the local list + */ + aws_linked_list_swap_contents(&manager->pending_acquisitions, &pending_acquisitions_to_fail); + + AWS_LOGF_INFO( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: manager release, failing %zu pending acquisitions", + (void *)manager, + manager->pending_acquisition_count); + manager->pending_acquisition_count = 0; + } + } else { + AWS_LOGF_ERROR( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: Connection manager release called with a zero reference count", + (void *)manager); } aws_mutex_unlock(&manager->lock); size_t connection_count = aws_array_list_length(&connections_to_release); + if (connection_count > 0) { + AWS_LOGF_INFO( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: manager release, releasing %zu held connections", + (void *)manager, + connection_count); + } for (size_t i = 0; i < connection_count; ++i) { struct aws_http_connection *connection = NULL; @@ -353,43 +483,39 @@ void aws_http_connection_manager_release(struct aws_http_connection_manager *man } } -static void s_aws_http_connection_manager_build_work_order( - struct aws_http_connection_manager *connection_manager, +static void s_aws_http_connection_manager_build_task_set( + struct aws_http_connection_manager *manager, struct aws_linked_list *completions, size_t *new_connections) { /* * Step 1 - If there's free connections, complete acquisition requests */ - while (aws_array_list_length(&connection_manager->connections) > 0 && - connection_manager->pending_acquisition_count > 0) { + while (aws_array_list_length(&manager->connections) > 0 && manager->pending_acquisition_count > 0) { struct aws_http_connection *connection = NULL; - aws_array_list_back(&connection_manager->connections, &connection); + aws_array_list_back(&manager->connections, &connection); - aws_array_list_pop_back(&connection_manager->connections); + aws_array_list_pop_back(&manager->connections); - s_aws_http_connection_manager_move_front_acquisition( - connection_manager, connection, AWS_ERROR_SUCCESS, completions); - ++connection_manager->vended_connection_count; + s_aws_http_connection_manager_move_front_acquisition(manager, connection, AWS_ERROR_SUCCESS, completions); + ++manager->vended_connection_count; } /* * Step 2 - if there's excess pending acquisitions and we have room to make more, make more */ - if (connection_manager->pending_acquisition_count > connection_manager->pending_connects_count) { - AWS_ASSERT( - connection_manager->max_connections >= - connection_manager->vended_connection_count + connection_manager->pending_connects_count); + if (manager->pending_acquisition_count > manager->pending_connects_count) { + AWS_FATAL_ASSERT( + manager->max_connections >= manager->vended_connection_count + manager->pending_connects_count); - *new_connections = connection_manager->pending_acquisition_count - connection_manager->pending_connects_count; + *new_connections = manager->pending_acquisition_count - manager->pending_connects_count; size_t max_new_connections = - connection_manager->max_connections - - (connection_manager->vended_connection_count + connection_manager->pending_connects_count); + manager->max_connections - (manager->vended_connection_count + manager->pending_connects_count); if (*new_connections > max_new_connections) { *new_connections = max_new_connections; } - connection_manager->pending_connects_count += *new_connections; + manager->pending_connects_count += *new_connections; } } @@ -397,35 +523,42 @@ static void s_aws_http_connection_manager_on_connection_setup( struct aws_http_connection *connection, int error_code, void *user_data); + static void s_aws_http_connection_manager_on_connection_shutdown( struct aws_http_connection *connection, int error_code, void *user_data); -static int s_aws_http_connection_manager_new_connection(struct aws_http_connection_manager *connection_manager) { +static int s_aws_http_connection_manager_new_connection(struct aws_http_connection_manager *manager) { struct aws_http_client_connection_options options; AWS_ZERO_STRUCT(options); options.self_size = sizeof(struct aws_http_client_connection_options); - options.bootstrap = connection_manager->bootstrap; - options.tls_options = connection_manager->tls_connection_options; - options.allocator = connection_manager->allocator; - options.user_data = connection_manager; - options.host_name = aws_byte_cursor_from_string(connection_manager->host); - options.port = connection_manager->port; - options.initial_window_size = connection_manager->initial_window_size; - options.socket_options = &connection_manager->socket_options; + options.bootstrap = manager->bootstrap; + options.tls_options = manager->tls_connection_options; + options.allocator = manager->allocator; + options.user_data = manager; + options.host_name = aws_byte_cursor_from_string(manager->host); + options.port = manager->port; + options.initial_window_size = manager->initial_window_size; + options.socket_options = &manager->socket_options; options.on_setup = s_aws_http_connection_manager_on_connection_setup; options.on_shutdown = s_aws_http_connection_manager_on_connection_shutdown; - if (connection_manager->function_table->create_connection(&options)) { + if (manager->function_table->create_connection(&options)) { + AWS_LOGF_ERROR( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: http connection creation failed with error code %d(%s)", + (void *)manager, + aws_last_error(), + aws_error_str(aws_last_error())); return AWS_OP_ERR; } return AWS_OP_SUCCESS; } -static void s_aws_http_connection_manager_execute_work_order( - struct aws_http_connection_manager *connection_manager, +static void s_aws_http_connection_manager_execute_task_set( + struct aws_http_connection_manager *manager, struct aws_linked_list *completions, size_t new_connections) { @@ -435,23 +568,37 @@ static void s_aws_http_connection_manager_execute_work_order( struct aws_array_list errors; AWS_ZERO_STRUCT(errors); - bool push_errors = aws_array_list_init_dynamic( - &errors, connection_manager->allocator, new_connections, sizeof(int)) == AWS_ERROR_SUCCESS; + if (new_connections > 0) { + AWS_LOGF_INFO( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: Requesting %zu new connections from http", + (void *)manager, + new_connections); + } + + /* Even if we can't init this array, we still need to invoke error callbacks properly */ + bool push_errors = + aws_array_list_init_dynamic(&errors, manager->allocator, new_connections, sizeof(int)) == AWS_ERROR_SUCCESS; + for (size_t i = 0; i < new_connections; ++i) { - if (s_aws_http_connection_manager_new_connection(connection_manager)) { + if (s_aws_http_connection_manager_new_connection(manager)) { ++new_connection_failures; representative_error = aws_last_error(); if (push_errors) { - aws_array_list_push_back(&errors, &representative_error); + AWS_FATAL_ASSERT(aws_array_list_push_back(&errors, &representative_error) == AWS_OP_SUCCESS); } } } if (new_connection_failures > 0) { - aws_mutex_lock(&connection_manager->lock); + /* + * We failed and aren't going to receive a callback, but the current state assumes we will receive + * a callback. So we need to re-lock and update the state ourselves. + */ + aws_mutex_lock(&manager->lock); - connection_manager->pending_connects_count -= new_connection_failures; - size_t i = 0; + AWS_FATAL_ASSERT(manager->pending_connects_count >= new_connection_failures); + manager->pending_connects_count -= new_connection_failures; /* * Rather than failing one acquisition for each connection failure, if there's at least one @@ -459,38 +606,43 @@ static void s_aws_http_connection_manager_execute_work_order( * connect that will necessarily resolve them. * * Try to correspond an error with the acquisition failure, but as a fallback just use the - * representative error + * representative error. */ - while (connection_manager->pending_acquisition_count > connection_manager->pending_connects_count) { + size_t i = 0; + while (manager->pending_acquisition_count > manager->pending_connects_count) { int error = representative_error; if (i < aws_array_list_length(&errors)) { aws_array_list_get_at(&errors, &error, i); } - s_aws_http_connection_manager_move_front_acquisition(connection_manager, NULL, error, completions); + s_aws_http_connection_manager_move_front_acquisition(manager, NULL, error, completions); ++i; } - aws_mutex_unlock(&connection_manager->lock); + aws_mutex_unlock(&manager->lock); } - s_aws_http_connection_manager_complete_acquisitions(completions, connection_manager->allocator); + s_aws_http_connection_manager_complete_acquisitions(completions, manager->allocator); aws_array_list_clean_up(&errors); } int aws_http_connection_manager_acquire_connection( - struct aws_http_connection_manager *connection_manager, + struct aws_http_connection_manager *manager, aws_http_on_client_connection_setup_fn *callback, void *user_data) { + int result = AWS_OP_ERR; + struct aws_http_connection_acquisition *request = - aws_mem_acquire(connection_manager->allocator, sizeof(struct aws_http_connection_acquisition)); + aws_mem_acquire(manager->allocator, sizeof(struct aws_http_connection_acquisition)); if (request == NULL) { callback(NULL, aws_last_error(), user_data); - return AWS_OP_ERR; + return result; } + AWS_LOGF_DEBUG(AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: Acquire connection", (void *)manager); + AWS_ZERO_STRUCT(*request); request->callback = callback; request->user_data = user_data; @@ -500,55 +652,82 @@ int aws_http_connection_manager_acquire_connection( size_t new_connections = 0; - aws_mutex_lock(&connection_manager->lock); + aws_mutex_lock(&manager->lock); - AWS_FATAL_ASSERT(connection_manager->state == AWS_HCMST_READY); + AWS_ASSERT(manager->state == AWS_HCMST_READY); + if (manager->state == AWS_HCMST_READY) { + aws_linked_list_push_back(&manager->pending_acquisitions, &request->node); + ++manager->pending_acquisition_count; - aws_linked_list_push_back(&connection_manager->pending_acquisitions, &request->node); - ++connection_manager->pending_acquisition_count; + s_aws_http_connection_manager_build_task_set(manager, &completions, &new_connections); + result = AWS_OP_SUCCESS; + } else { + AWS_LOGF_ERROR( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: Acquire connection called when manager in shut down state", + (void *)manager); - s_aws_http_connection_manager_build_work_order(connection_manager, &completions, &new_connections); + request->error_code = AWS_ERROR_HTTP_CONNECTION_MANAGER_INVALID_STATE_FOR_ACQUIRE; + aws_linked_list_push_back(&completions, &request->node); - aws_mutex_unlock(&connection_manager->lock); + aws_raise_error(AWS_ERROR_HTTP_CONNECTION_MANAGER_INVALID_STATE_FOR_ACQUIRE); + } - s_aws_http_connection_manager_execute_work_order(connection_manager, &completions, new_connections); + aws_mutex_unlock(&manager->lock); - return AWS_OP_SUCCESS; + s_aws_http_connection_manager_execute_task_set(manager, &completions, new_connections); + + return result; } int aws_http_connection_manager_release_connection( - struct aws_http_connection_manager *connection_manager, + struct aws_http_connection_manager *manager, struct aws_http_connection *connection) { struct aws_linked_list completions; aws_linked_list_init(&completions); + int result = AWS_OP_ERR; size_t new_connections = 0; + bool should_release_connection = !manager->function_table->is_connection_open(connection); + + AWS_LOGF_DEBUG( + AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: Releasing connection (id=%p)", (void *)manager, (void *)connection); + + aws_mutex_lock(&manager->lock); - aws_mutex_lock(&connection_manager->lock); + /* We're probably hosed in this case, but let's not underflow */ + if (manager->vended_connection_count == 0) { + AWS_LOGF_ERROR( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: Connection released when vended connection count is zero", + (void *)manager); + aws_raise_error(AWS_ERROR_HTTP_CONNECTION_MANAGER_VENDED_CONNECTION_UNDERFLOW); + goto release; + } - AWS_FATAL_ASSERT(connection_manager->state == AWS_HCMST_READY); + result = AWS_OP_SUCCESS; - AWS_ASSERT(connection_manager->vended_connection_count > 0); - --connection_manager->vended_connection_count; + --manager->vended_connection_count; - bool should_release_connection = !connection_manager->function_table->is_connection_open(connection); if (!should_release_connection) { - if (aws_array_list_push_back(&connection_manager->connections, &connection)) { + if (aws_array_list_push_back(&manager->connections, &connection)) { should_release_connection = true; } } - s_aws_http_connection_manager_build_work_order(connection_manager, &completions, &new_connections); + s_aws_http_connection_manager_build_task_set(manager, &completions, &new_connections); - aws_mutex_unlock(&connection_manager->lock); +release: - s_aws_http_connection_manager_execute_work_order(connection_manager, &completions, new_connections); + aws_mutex_unlock(&manager->lock); + + s_aws_http_connection_manager_execute_task_set(manager, &completions, new_connections); if (should_release_connection) { - connection_manager->function_table->release_connection(connection); + manager->function_table->release_connection(connection); } - return AWS_OP_SUCCESS; + return result; } static void s_aws_http_connection_manager_on_connection_setup( @@ -562,12 +741,26 @@ static void s_aws_http_connection_manager_on_connection_setup( size_t new_connections = 0; + if (connection != NULL) { + AWS_LOGF_DEBUG( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: Received new connection (id=%p) from http layer", + (void *)manager, + (void *)connection); + } else { + AWS_LOGF_WARN( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: Failed to obtain new connection from http layer, error %d(%s)", + (void *)manager, + error_code, + aws_error_str(error_code)); + } + aws_mutex_lock(&manager->lock); bool is_shutting_down = manager->state == AWS_HCMST_SHUTTING_DOWN; - AWS_ASSERT(manager->pending_acquisition_count == 0 || !is_shutting_down); - AWS_ASSERT(manager->pending_connects_count > 0); + AWS_FATAL_ASSERT(manager->pending_connects_count > 0); --manager->pending_connects_count; if (connection != NULL) { @@ -576,11 +769,7 @@ static void s_aws_http_connection_manager_on_connection_setup( AWS_FATAL_ASSERT(aws_array_list_push_back(&manager->connections, &connection) == AWS_OP_SUCCESS); } ++manager->open_connection_count; - } - - bool should_destroy = s_aws_http_connection_manager_should_destroy(manager); - - if (connection == NULL) { + } else { /* * To be safe, if we have an excess of pending acquisitions (beyond the number of pending * connects), we need to fail all of the excess. Technically, we might be able to try and @@ -593,16 +782,23 @@ static void s_aws_http_connection_manager_on_connection_setup( } } - s_aws_http_connection_manager_build_work_order(manager, &completions, &new_connections); + s_aws_http_connection_manager_build_task_set(manager, &completions, &new_connections); + + bool should_destroy = s_aws_http_connection_manager_should_destroy(manager); aws_mutex_unlock(&manager->lock); - s_aws_http_connection_manager_execute_work_order(manager, &completions, new_connections); + s_aws_http_connection_manager_execute_task_set(manager, &completions, new_connections); if (is_shutting_down && connection != NULL) { /* * We didn't add the connection to the pool; just release it immediately */ + AWS_LOGF_DEBUG( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: New connection (id=%p) releasing immediately due to shutdown state", + (void *)manager, + (void *)connection); manager->function_table->release_connection(connection); } @@ -620,18 +816,30 @@ static void s_aws_http_connection_manager_on_connection_shutdown( bool should_release_connection = false; struct aws_http_connection_manager *manager = user_data; + + AWS_LOGF_DEBUG( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: shutdown received for connection (id=%p)", + (void *)manager, + (void *)connection); + aws_mutex_lock(&manager->lock); + AWS_FATAL_ASSERT(manager->open_connection_count > 0); --manager->open_connection_count; bool should_destroy = s_aws_http_connection_manager_should_destroy(manager); size_t connection_count = aws_array_list_length(&manager->connections); + /* + * Find and, if found, remove it from connections + */ if (connection_count > 0) { AWS_ASSERT(manager->state == AWS_HCMST_READY); struct aws_http_connection *last_connection = NULL; - aws_array_list_get_at(&manager->connections, &last_connection, connection_count - 1); + AWS_FATAL_ASSERT( + aws_array_list_get_at(&manager->connections, &last_connection, connection_count - 1) == AWS_OP_SUCCESS); for (size_t i = 0; i < connection_count; ++i) { struct aws_http_connection *current_connection = NULL; @@ -651,9 +859,14 @@ static void s_aws_http_connection_manager_on_connection_shutdown( aws_mutex_unlock(&manager->lock); - AWS_ASSERT(!should_release_connection || !should_destroy); + AWS_FATAL_ASSERT(!should_release_connection || !should_destroy); if (should_release_connection) { + AWS_LOGF_INFO( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: Releasing held connection (id=%p)", + (void *)manager, + (void *)connection); manager->function_table->release_connection(connection); } diff --git a/source/http.c b/source/http.c index f02c91e95..41da6076f 100644 --- a/source/http.c +++ b/source/http.c @@ -55,6 +55,12 @@ static struct aws_error_info s_errors[] = { AWS_DEFINE_ERROR_INFO_HTTP( AWS_ERROR_HTTP_OUTGOING_STREAM_LENGTH_INCORRECT, "Amount of data streamed out does not match the previously declared length."), + AWS_DEFINE_ERROR_INFO_HTTP( + AWS_ERROR_HTTP_CONNECTION_MANAGER_INVALID_STATE_FOR_ACQUIRE, + "Acquire called after the connection manager's ref count has reached zero"), + AWS_DEFINE_ERROR_INFO_HTTP( + AWS_ERROR_HTTP_CONNECTION_MANAGER_VENDED_CONNECTION_UNDERFLOW, + "Release called when the connection manager's vended connection count was zero"), }; /* clang-format on */ @@ -68,7 +74,7 @@ static struct aws_log_subject_info s_log_subject_infos[] = { DEFINE_LOG_SUBJECT_INFO(AWS_LS_HTTP_CONNECTION, "http-connection", "HTTP client or server connection"), DEFINE_LOG_SUBJECT_INFO(AWS_LS_HTTP_SERVER, "http-server", "HTTP server socket listening for incoming connections"), DEFINE_LOG_SUBJECT_INFO(AWS_LS_HTTP_STREAM, "http-stream", "HTTP request-response exchange"), -}; + DEFINE_LOG_SUBJECT_INFO(AWS_LS_HTTP_CONNECTION_MANAGER, "connection-manager", "Http connection manager")}; static struct aws_log_subject_info_list s_log_subject_list = { .subject_list = s_log_subject_infos, From 02c3400b08574c07dffa93379eba244046b8faad Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Wed, 15 May 2019 13:56:42 -0700 Subject: [PATCH 08/14] Formatting --- source/http.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/http.c b/source/http.c index ccd36e3c3..a453c0e27 100644 --- a/source/http.c +++ b/source/http.c @@ -62,11 +62,11 @@ static struct aws_error_info s_errors[] = { AWS_ERROR_HTTP_END_RANGE, "Not a real error and should never be seen."), AWS_DEFINE_ERROR_INFO_HTTP( - AWS_ERROR_HTTP_CONNECTION_MANAGER_INVALID_STATE_FOR_ACQUIRE, - "Acquire called after the connection manager's ref count has reached zero"), + AWS_ERROR_HTTP_CONNECTION_MANAGER_INVALID_STATE_FOR_ACQUIRE, + "Acquire called after the connection manager's ref count has reached zero"), AWS_DEFINE_ERROR_INFO_HTTP( - AWS_ERROR_HTTP_CONNECTION_MANAGER_VENDED_CONNECTION_UNDERFLOW, - "Release called when the connection manager's vended connection count was zero"), + AWS_ERROR_HTTP_CONNECTION_MANAGER_VENDED_CONNECTION_UNDERFLOW, + "Release called when the connection manager's vended connection count was zero"), }; /* clang-format on */ From d3de76015467d5a98dd82b5d61d7a8895ba38f0f Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Thu, 16 May 2019 12:52:31 -0700 Subject: [PATCH 09/14] clang tidy/logic --- tests/test_connection_manager.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c index 33deeaef3..db9c2763e 100644 --- a/tests/test_connection_manager.c +++ b/tests/test_connection_manager.c @@ -413,13 +413,19 @@ static int s_aws_http_connection_manager_create_connection_sync_mock( aws_array_list_get_at(&tester->mock_connections, &connection, next_connection_id); } - if (connection->result == AWS_NCRT_SUCCESS) { - options->on_setup((struct aws_http_connection *)connection, AWS_ERROR_SUCCESS, options->user_data); - } else if (connection->result == AWS_NCRT_ERROR_VIA_CALLBACK) { - options->on_setup(NULL, AWS_ERROR_HTTP_UNKNOWN, options->user_data); + if (connection) { + if (connection->result == AWS_NCRT_SUCCESS) { + options->on_setup((struct aws_http_connection *)connection, AWS_ERROR_SUCCESS, options->user_data); + } else if (connection->result == AWS_NCRT_ERROR_VIA_CALLBACK) { + options->on_setup(NULL, AWS_ERROR_HTTP_UNKNOWN, options->user_data); + } + + if (connection->result != AWS_NCRT_ERROR_FROM_CREATE) { + return AWS_OP_SUCCESS; + } } - return connection->result != AWS_NCRT_ERROR_FROM_CREATE ? AWS_ERROR_SUCCESS : AWS_ERROR_HTTP_UNKNOWN; + return aws_raise_error(AWS_ERROR_HTTP_UNKNOWN); } static void s_aws_http_connection_manager_release_connection_sync_mock(struct aws_http_connection *connection) { From 40aa1fd251465a7c0e95e7ea107ece0436d47c3c Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Thu, 16 May 2019 13:47:16 -0700 Subject: [PATCH 10/14] more clang tidy --- tests/test_connection_manager.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c index db9c2763e..33af902d5 100644 --- a/tests/test_connection_manager.c +++ b/tests/test_connection_manager.c @@ -158,6 +158,10 @@ void s_release_connections(size_t count, bool close_first) { release_count = count; } + if (release_count == 0) { + return; + } + struct aws_array_list to_release; aws_array_list_init_dynamic(&to_release, tester->allocator, release_count, sizeof(struct aws_http_connection *)); @@ -200,6 +204,9 @@ void s_release_connections(size_t count, bool close_first) { } void s_on_acquire_connection(struct aws_http_connection *connection, int error_code, void *user_data) { + (void)error_code; + (void)user_data; + struct cm_tester *tester = &s_tester; aws_mutex_lock(&tester->lock); @@ -224,6 +231,8 @@ static void s_acquire_connections(size_t count) { } static bool s_is_connection_reply_count_at_least(void *context) { + (void)context; + struct cm_tester *tester = &s_tester; return tester->wait_for_connection_count <= @@ -405,7 +414,7 @@ static int s_aws_http_connection_manager_create_connection_sync_mock( struct cm_tester *tester = &s_tester; size_t next_connection_id = aws_atomic_fetch_add(&tester->next_connection_id, 1); - aws_atomic_store_ptr(&tester->release_connection_fn, options->on_shutdown); + aws_atomic_store_ptr(&tester->release_connection_fn, (void *)options->on_shutdown); struct mock_connection_proxy *connection = NULL; From 20aa022facb4b881f8aae6acfb0c91bf85b11c30 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Thu, 16 May 2019 14:06:48 -0700 Subject: [PATCH 11/14] tidy --- tests/test_connection_manager.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c index 33af902d5..b20d68f19 100644 --- a/tests/test_connection_manager.c +++ b/tests/test_connection_manager.c @@ -163,7 +163,10 @@ void s_release_connections(size_t count, bool close_first) { } struct aws_array_list to_release; - aws_array_list_init_dynamic(&to_release, tester->allocator, release_count, sizeof(struct aws_http_connection *)); + if (aws_array_list_init_dynamic( + &to_release, tester->allocator, release_count, sizeof(struct aws_http_connection *))) { + return; + } for (size_t i = 0; i < release_count; ++i) { struct aws_http_connection *connection = NULL; From 006d11ae2868f647560cf9d6c18f78b459d90364 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Thu, 16 May 2019 14:38:44 -0700 Subject: [PATCH 12/14] Function pointer messiness --- tests/test_connection_manager.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c index b20d68f19..1dd769474 100644 --- a/tests/test_connection_manager.c +++ b/tests/test_connection_manager.c @@ -69,7 +69,7 @@ struct cm_tester { struct aws_atomic_var next_connection_id; struct aws_array_list mock_connections; - struct aws_atomic_var release_connection_fn; + aws_http_on_client_connection_shutdown_fn *release_connection_fn; }; static struct cm_tester s_tester; @@ -125,7 +125,6 @@ int s_cm_tester_init(struct cm_tester_options *options) { tester->mock_table = options->mock_table; aws_atomic_store_int(&tester->next_connection_id, 0); - aws_atomic_store_ptr(&tester->release_connection_fn, NULL); ASSERT_SUCCESS(aws_array_list_init_dynamic( &tester->mock_connections, tester->allocator, 10, sizeof(struct mock_connection_proxy *))); @@ -417,7 +416,10 @@ static int s_aws_http_connection_manager_create_connection_sync_mock( struct cm_tester *tester = &s_tester; size_t next_connection_id = aws_atomic_fetch_add(&tester->next_connection_id, 1); - aws_atomic_store_ptr(&tester->release_connection_fn, (void *)options->on_shutdown); + + aws_mutex_lock(&tester->lock); + tester->release_connection_fn = options->on_shutdown; + aws_mutex_unlock(&tester->lock); struct mock_connection_proxy *connection = NULL; @@ -445,9 +447,7 @@ static void s_aws_http_connection_manager_release_connection_sync_mock(struct aw struct cm_tester *tester = &s_tester; - aws_http_on_client_connection_shutdown_fn *release_callback = aws_atomic_load_ptr(&tester->release_connection_fn); - - release_callback(connection, AWS_ERROR_SUCCESS, tester->connection_manager); + tester->release_connection_fn(connection, AWS_ERROR_SUCCESS, tester->connection_manager); } static void s_aws_http_connection_manager_close_connection_sync_mock(struct aws_http_connection *connection) { From 2451c6f72ef9db5b85d0f37b16cbdd2a7a172933 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Mon, 20 May 2019 16:04:16 -0700 Subject: [PATCH 13/14] WIP --- include/aws/http/connection_manager.h | 9 ++++++--- source/connection_manager.c | 4 ++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h index 2f8498465..6074f427d 100644 --- a/include/aws/http/connection_manager.h +++ b/include/aws/http/connection_manager.h @@ -19,13 +19,16 @@ #include #include -#include struct aws_client_bootstrap; +struct aws_http_connection; struct aws_http_connection_manager; +struct aws_http_connection_manager_mocks; struct aws_socket_options; struct aws_tls_connection_options; -struct aws_http_connection_manager_mocks; + +typedef void( + aws_http_connection_manager_on_connection_setup_fn)(struct aws_http_connection *connection, int error_code, void *user_data); /* * Connection manager configuration struct. @@ -84,7 +87,7 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( AWS_HTTP_API int aws_http_connection_manager_acquire_connection( struct aws_http_connection_manager *manager, - aws_http_on_client_connection_setup_fn *callback, + aws_http_connection_manager_on_connection_setup_fn *callback, void *user_data); /* diff --git a/source/connection_manager.c b/source/connection_manager.c index 685e9af7e..ccc03ed82 100644 --- a/source/connection_manager.c +++ b/source/connection_manager.c @@ -225,7 +225,7 @@ static bool s_aws_http_connection_manager_should_destroy(struct aws_http_connect struct aws_http_connection_acquisition { struct aws_linked_list_node node; struct aws_http_connection_manager *manager; /* Only used by logging */ - aws_http_on_client_connection_setup_fn *callback; + aws_http_connection_manager_on_connection_setup_fn *callback; void *user_data; struct aws_http_connection *connection; int error_code; @@ -629,7 +629,7 @@ static void s_aws_http_connection_manager_execute_task_set( int aws_http_connection_manager_acquire_connection( struct aws_http_connection_manager *manager, - aws_http_on_client_connection_setup_fn *callback, + aws_http_connection_manager_on_connection_setup_fn *callback, void *user_data) { int result = AWS_OP_ERR; From cb447e612b60f5ed58ef55ca043a725c7a00a454 Mon Sep 17 00:00:00 2001 From: Bret Ambrose Date: Tue, 21 May 2019 11:57:29 -0700 Subject: [PATCH 14/14] CR Feedback + snapshot logging --- include/aws/http/connection_manager.h | 28 ++-- .../connection_manager_function_table.h | 12 +- source/connection_manager.c | 131 +++++++++++++++--- tests/test_connection_manager.c | 7 +- 4 files changed, 142 insertions(+), 36 deletions(-) diff --git a/include/aws/http/connection_manager.h b/include/aws/http/connection_manager.h index 6074f427d..a89a919b3 100644 --- a/include/aws/http/connection_manager.h +++ b/include/aws/http/connection_manager.h @@ -27,8 +27,10 @@ struct aws_http_connection_manager_mocks; struct aws_socket_options; struct aws_tls_connection_options; -typedef void( - aws_http_connection_manager_on_connection_setup_fn)(struct aws_http_connection *connection, int error_code, void *user_data); +typedef void(aws_http_connection_manager_on_connection_setup_fn)( + struct aws_http_connection *connection, + int error_code, + void *user_data); /* * Connection manager configuration struct. @@ -51,11 +53,6 @@ struct aws_http_connection_manager_options { * Maximum number of connections this manager is allowed to contain */ size_t max_connections; - - /* - * Private function table for mocking http connection management - */ - const struct aws_http_connection_manager_function_table *function_table; }; AWS_EXTERN_C_BEGIN @@ -68,12 +65,20 @@ void aws_http_connection_manager_acquire(struct aws_http_connection_manager *man /* * Connection managers are ref counted. Removes one external ref from the manager. + * + * When the ref count goes to zero, the connection manager begins its shut down + * process. All pending connection acquisitions are failed (with callbacks + * invoked) and any (erroneous) subsequent attempts to acquire a connection + * fail immediately. The connection manager destroys itself once all pending + * asynchronous activities have resolved. */ AWS_HTTP_API void aws_http_connection_manager_release(struct aws_http_connection_manager *manager); /* * Creates a new connection manager with the supplied configuration options. + * + * The returned connection manager begins with a ref count of 1. */ AWS_HTTP_API struct aws_http_connection_manager *aws_http_connection_manager_new( @@ -83,15 +88,20 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( /* * Requests a connection from the manager. The requester is notified of * an acquired connection (or failure to acquire) via the supplied callback. + * + * Once a connection has been successfully acquired from the manager it + * must be released back (via aws_http_connection_manager_release_connection) + * at some point. Failure to do so will cause a resource leak. */ AWS_HTTP_API -int aws_http_connection_manager_acquire_connection( +void aws_http_connection_manager_acquire_connection( struct aws_http_connection_manager *manager, aws_http_connection_manager_on_connection_setup_fn *callback, void *user_data); /* - * Returns a connection back to the manager + * Returns a connection back to the manager. All acquired connections must + * eventually be released back to the manager in order to avoid a resource leak. */ AWS_HTTP_API int aws_http_connection_manager_release_connection( diff --git a/include/aws/http/private/connection_manager_function_table.h b/include/aws/http/private/connection_manager_function_table.h index 03b063745..d815d36ca 100644 --- a/include/aws/http/private/connection_manager_function_table.h +++ b/include/aws/http/private/connection_manager_function_table.h @@ -35,12 +35,14 @@ struct aws_http_connection_manager_function_table { aws_http_connection_manager_is_connection_open_fn *is_connection_open; }; -AWS_STATIC_IMPL +AWS_HTTP_API bool aws_http_connection_manager_function_table_is_valid( - const struct aws_http_connection_manager_function_table *table) { - return table->create_connection && table->close_connection && table->release_connection && - table->is_connection_open; -} + const struct aws_http_connection_manager_function_table *table); + +AWS_HTTP_API +void aws_http_connection_manager_set_function_table( + struct aws_http_connection_manager *manager, + const struct aws_http_connection_manager_function_table *function_table); AWS_HTTP_API extern const struct aws_http_connection_manager_function_table diff --git a/source/connection_manager.c b/source/connection_manager.c index ccc03ed82..6c226e7aa 100644 --- a/source/connection_manager.c +++ b/source/connection_manager.c @@ -38,7 +38,13 @@ static struct aws_http_connection_manager_function_table s_default_function_tabl const struct aws_http_connection_manager_function_table *g_aws_http_connection_manager_default_function_table_ptr = &s_default_function_table; -enum aws_http_connection_manager_state_type { AWS_HCMST_READY, AWS_HCMST_SHUTTING_DOWN }; +bool aws_http_connection_manager_function_table_is_valid( + const struct aws_http_connection_manager_function_table *table) { + return table->create_connection && table->close_connection && table->release_connection && + table->is_connection_open; +} + +enum aws_http_connection_manager_state_type { AWS_HCMST_UNINITIALIZED, AWS_HCMST_READY, AWS_HCMST_SHUTTING_DOWN }; /** * Vocabulary @@ -186,6 +192,65 @@ struct aws_http_connection_manager { size_t external_ref_count; }; +struct aws_http_connection_manager_snapshot { + enum aws_http_connection_manager_state_type state; + + size_t held_connection_count; + size_t pending_acquisition_count; + size_t pending_connects_count; + size_t vended_connection_count; + size_t open_connection_count; + + size_t external_ref_count; +}; + +/* + * Correct usage requires AWS_ZERO_STRUCT to have been called beforehand. + */ +static void s_aws_http_connection_manager_get_snapshot( + struct aws_http_connection_manager *manager, + struct aws_http_connection_manager_snapshot *snapshot) { + + snapshot->state = manager->state; + snapshot->held_connection_count = aws_array_list_length(&manager->connections); + snapshot->pending_acquisition_count = manager->pending_acquisition_count; + snapshot->pending_connects_count = manager->pending_connects_count; + snapshot->vended_connection_count = manager->vended_connection_count; + snapshot->open_connection_count = manager->open_connection_count; + + snapshot->external_ref_count = manager->external_ref_count; +} + +static void s_aws_http_connection_manager_log_snapshot( + struct aws_http_connection_manager *manager, + struct aws_http_connection_manager_snapshot *snapshot) { + if (snapshot->state != AWS_HCMST_UNINITIALIZED) { + AWS_LOGF_DEBUG( + AWS_LS_HTTP_CONNECTION_MANAGER, + "id=%p: snapshot - state=%d, held_connection_count=%zu, pending_acquire_count=%zu, " + "pending_connect_count=%zu, vended_connection_count=%zu, open_connection_count=%zu, ref_count=%zu", + (void *)manager, + (int)snapshot->state, + snapshot->held_connection_count, + snapshot->pending_acquisition_count, + snapshot->pending_connects_count, + snapshot->vended_connection_count, + snapshot->open_connection_count, + snapshot->external_ref_count); + } else { + AWS_LOGF_DEBUG( + AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: snapshot not initialized by control flow", (void *)manager); + } +} + +void aws_http_connection_manager_set_function_table( + struct aws_http_connection_manager *manager, + const struct aws_http_connection_manager_function_table *function_table) { + AWS_FATAL_ASSERT(aws_http_connection_manager_function_table_is_valid(function_table)); + + manager->function_table = function_table; +} + /* * Hard Requirement: Manager's lock must held somewhere in the call stack */ @@ -375,15 +440,8 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( manager->max_connections = options->max_connections; manager->socket_options = *options->socket_options; manager->bootstrap = options->bootstrap; - manager->function_table = options->function_table; + manager->function_table = g_aws_http_connection_manager_default_function_table_ptr; manager->external_ref_count = 1; - if (manager->function_table == NULL) { - manager->function_table = g_aws_http_connection_manager_default_function_table_ptr; - } - - if (!aws_http_connection_manager_function_table_is_valid(manager->function_table)) { - goto on_error; - } AWS_LOGF_INFO(AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: Successfully created", (void *)manager); @@ -398,6 +456,7 @@ struct aws_http_connection_manager *aws_http_connection_manager_new( void aws_http_connection_manager_acquire(struct aws_http_connection_manager *manager) { aws_mutex_lock(&manager->lock); + AWS_FATAL_ASSERT(manager->external_ref_count > 0); manager->external_ref_count += 1; aws_mutex_unlock(&manager->lock); } @@ -487,6 +546,9 @@ static void s_aws_http_connection_manager_build_task_set( struct aws_http_connection_manager *manager, struct aws_linked_list *completions, size_t *new_connections) { + + *new_connections = 0; + /* * Step 1 - If there's free connections, complete acquisition requests */ @@ -627,18 +689,16 @@ static void s_aws_http_connection_manager_execute_task_set( aws_array_list_clean_up(&errors); } -int aws_http_connection_manager_acquire_connection( +void aws_http_connection_manager_acquire_connection( struct aws_http_connection_manager *manager, aws_http_connection_manager_on_connection_setup_fn *callback, void *user_data) { - int result = AWS_OP_ERR; - struct aws_http_connection_acquisition *request = aws_mem_acquire(manager->allocator, sizeof(struct aws_http_connection_acquisition)); if (request == NULL) { callback(NULL, aws_last_error(), user_data); - return result; + return; } AWS_LOGF_DEBUG(AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: Acquire connection", (void *)manager); @@ -652,15 +712,16 @@ int aws_http_connection_manager_acquire_connection( size_t new_connections = 0; + struct aws_http_connection_manager_snapshot snapshot; + AWS_ZERO_STRUCT(snapshot); + aws_mutex_lock(&manager->lock); - AWS_ASSERT(manager->state == AWS_HCMST_READY); if (manager->state == AWS_HCMST_READY) { aws_linked_list_push_back(&manager->pending_acquisitions, &request->node); ++manager->pending_acquisition_count; s_aws_http_connection_manager_build_task_set(manager, &completions, &new_connections); - result = AWS_OP_SUCCESS; } else { AWS_LOGF_ERROR( AWS_LS_HTTP_CONNECTION_MANAGER, @@ -673,11 +734,13 @@ int aws_http_connection_manager_acquire_connection( aws_raise_error(AWS_ERROR_HTTP_CONNECTION_MANAGER_INVALID_STATE_FOR_ACQUIRE); } + s_aws_http_connection_manager_get_snapshot(manager, &snapshot); + aws_mutex_unlock(&manager->lock); - s_aws_http_connection_manager_execute_task_set(manager, &completions, new_connections); + s_aws_http_connection_manager_log_snapshot(manager, &snapshot); - return result; + s_aws_http_connection_manager_execute_task_set(manager, &completions, new_connections); } int aws_http_connection_manager_release_connection( @@ -686,10 +749,14 @@ int aws_http_connection_manager_release_connection( struct aws_linked_list completions; aws_linked_list_init(&completions); + bool should_destroy = false; int result = AWS_OP_ERR; size_t new_connections = 0; bool should_release_connection = !manager->function_table->is_connection_open(connection); + struct aws_http_connection_manager_snapshot snapshot; + AWS_ZERO_STRUCT(snapshot); + AWS_LOGF_DEBUG( AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: Releasing connection (id=%p)", (void *)manager, (void *)connection); @@ -697,7 +764,7 @@ int aws_http_connection_manager_release_connection( /* We're probably hosed in this case, but let's not underflow */ if (manager->vended_connection_count == 0) { - AWS_LOGF_ERROR( + AWS_LOGF_FATAL( AWS_LS_HTTP_CONNECTION_MANAGER, "id=%p: Connection released when vended connection count is zero", (void *)manager); @@ -717,16 +784,29 @@ int aws_http_connection_manager_release_connection( s_aws_http_connection_manager_build_task_set(manager, &completions, &new_connections); + /* + * This could be the last connection and we might have already gotten the release callback + * from http. In that case, this would be our last chance to detect a destroyable state. + */ + should_destroy = s_aws_http_connection_manager_should_destroy(manager); + s_aws_http_connection_manager_get_snapshot(manager, &snapshot); + release: aws_mutex_unlock(&manager->lock); + s_aws_http_connection_manager_log_snapshot(manager, &snapshot); + s_aws_http_connection_manager_execute_task_set(manager, &completions, new_connections); if (should_release_connection) { manager->function_table->release_connection(connection); } + if (should_destroy) { + s_aws_http_connection_manager_destroy(manager); + } + return result; } @@ -756,6 +836,9 @@ static void s_aws_http_connection_manager_on_connection_setup( aws_error_str(error_code)); } + struct aws_http_connection_manager_snapshot snapshot; + AWS_ZERO_STRUCT(snapshot); + aws_mutex_lock(&manager->lock); bool is_shutting_down = manager->state == AWS_HCMST_SHUTTING_DOWN; @@ -785,9 +868,12 @@ static void s_aws_http_connection_manager_on_connection_setup( s_aws_http_connection_manager_build_task_set(manager, &completions, &new_connections); bool should_destroy = s_aws_http_connection_manager_should_destroy(manager); + s_aws_http_connection_manager_get_snapshot(manager, &snapshot); aws_mutex_unlock(&manager->lock); + s_aws_http_connection_manager_log_snapshot(manager, &snapshot); + s_aws_http_connection_manager_execute_task_set(manager, &completions, new_connections); if (is_shutting_down && connection != NULL) { @@ -823,11 +909,13 @@ static void s_aws_http_connection_manager_on_connection_shutdown( (void *)manager, (void *)connection); + struct aws_http_connection_manager_snapshot snapshot; + AWS_ZERO_STRUCT(snapshot); + aws_mutex_lock(&manager->lock); AWS_FATAL_ASSERT(manager->open_connection_count > 0); --manager->open_connection_count; - bool should_destroy = s_aws_http_connection_manager_should_destroy(manager); size_t connection_count = aws_array_list_length(&manager->connections); @@ -857,9 +945,12 @@ static void s_aws_http_connection_manager_on_connection_shutdown( } } + bool should_destroy = s_aws_http_connection_manager_should_destroy(manager); + s_aws_http_connection_manager_get_snapshot(manager, &snapshot); + aws_mutex_unlock(&manager->lock); - AWS_FATAL_ASSERT(!should_release_connection || !should_destroy); + s_aws_http_connection_manager_log_snapshot(manager, &snapshot); if (should_release_connection) { AWS_LOGF_INFO( diff --git a/tests/test_connection_manager.c b/tests/test_connection_manager.c index 1dd769474..67e65734c 100644 --- a/tests/test_connection_manager.c +++ b/tests/test_connection_manager.c @@ -116,12 +116,15 @@ int s_cm_tester_init(struct cm_tester_options *options) { NULL, //&tester->tls_connection_options, .host = aws_byte_cursor_from_c_str("www.google.com"), .port = 80, - .max_connections = options->max_connections, - .function_table = options->mock_table}; + .max_connections = options->max_connections}; tester->connection_manager = aws_http_connection_manager_new(tester->allocator, &cm_options); ASSERT_NOT_NULL(tester->connection_manager); + if (options->mock_table) { + aws_http_connection_manager_set_function_table(tester->connection_manager, options->mock_table); + } + tester->mock_table = options->mock_table; aws_atomic_store_int(&tester->next_connection_id, 0);