remove sba allocator (#145)

* remove sba allocator * leak on hash map
awslabs · Jul 29, 2021 · 071f0de · 071f0de
1 parent dca8df1
commit 071f0de
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 34 deletions.
diff --git a/include/aws/s3/private/s3_client_impl.h b/include/aws/s3/private/s3_client_impl.h
@@ -137,9 +137,6 @@ struct aws_s3_client_vtable {
 struct aws_s3_client {
     struct aws_allocator *allocator;
 
-    /* Small block allocator for our small allocations. */
-    struct aws_allocator *sba_allocator;
-
     struct aws_s3_client_vtable *vtable;
 
     struct aws_ref_count ref_count;

diff --git a/source/s3_client.c b/source/s3_client.c
@@ -242,11 +242,6 @@ struct aws_s3_client *aws_s3_client_new(
     struct aws_s3_client *client = aws_mem_calloc(allocator, 1, sizeof(struct aws_s3_client));
 
     client->allocator = allocator;
-    client->sba_allocator = aws_small_block_allocator_new(allocator, true);
-    if (!client->sba_allocator) {
-        goto sba_allocator_fail;
-    }
-
     client->vtable = &s_s3_client_default_vtable;
 
     aws_ref_count_init(&client->ref_count, client, (aws_simple_completion_callback *)s_s3_client_start_destroy);
@@ -396,7 +391,7 @@ struct aws_s3_client *aws_s3_client_new(
 
     aws_hash_table_init(
         &client->synced_data.endpoints,
-        client->sba_allocator,
+        client->allocator,
         10,
         aws_hash_string,
         aws_hash_callback_string_eq,
@@ -422,8 +417,6 @@ struct aws_s3_client *aws_s3_client_new(
     aws_client_bootstrap_release(client->client_bootstrap);
     aws_mutex_clean_up(&client->synced_data.lock);
 lock_init_fail:
-    aws_small_block_allocator_destroy(client->sba_allocator);
-sba_allocator_fail:
     aws_mem_release(client->allocator, client);
     return NULL;
 }
@@ -490,6 +483,7 @@ static void s_s3_client_finish_destroy_default(struct aws_s3_client *client) {
 
     AWS_ASSERT(aws_linked_list_empty(&client->synced_data.pending_meta_request_work));
     AWS_ASSERT(aws_linked_list_empty(&client->threaded_data.meta_requests));
+    aws_hash_table_clean_up(&client->synced_data.endpoints);
 
     aws_retry_strategy_release(client->retry_strategy);
 
@@ -498,8 +492,6 @@ static void s_s3_client_finish_destroy_default(struct aws_s3_client *client) {
     aws_client_bootstrap_release(client->client_bootstrap);
     aws_cached_signing_config_destroy(client->cached_signing_config);
 
-    aws_small_block_allocator_destroy(client->sba_allocator);
-
     aws_s3_client_shutdown_complete_callback_fn *shutdown_callback = client->shutdown_callback;
     void *shutdown_user_data = client->shutdown_callback_user_data;
 
@@ -627,7 +619,7 @@ struct aws_s3_meta_request *aws_s3_client_make_meta_request(
 
     aws_s3_client_lock_synced_data(client);
 
-    struct aws_string *endpoint_host_name = aws_string_new_from_cursor(client->sba_allocator, &host_header_value);
+    struct aws_string *endpoint_host_name = aws_string_new_from_cursor(client->allocator, &host_header_value);
 
     struct aws_s3_endpoint *endpoint = NULL;
     struct aws_hash_element *endpoint_hash_element = NULL;
@@ -652,7 +644,7 @@ struct aws_s3_meta_request *aws_s3_client_make_meta_request(
             .max_connections = aws_s3_client_get_max_active_connections(client, NULL),
         };
 
-        endpoint = aws_s3_endpoint_new(client->sba_allocator, &endpoint_options);
+        endpoint = aws_s3_endpoint_new(client->allocator, &endpoint_options);
 
         if (endpoint == NULL) {
             aws_hash_table_remove(&client->synced_data.endpoints, endpoint_host_name, NULL, NULL);
@@ -871,7 +863,7 @@ static void s_s3_client_push_meta_request_synced(
     ASSERT_SYNCED_DATA_LOCK_HELD(client);
 
     struct aws_s3_meta_request_work *meta_request_work =
-        aws_mem_calloc(client->sba_allocator, 1, sizeof(struct aws_s3_meta_request_work));
+        aws_mem_calloc(client->allocator, 1, sizeof(struct aws_s3_meta_request_work));
 
     aws_s3_meta_request_acquire(meta_request);
     meta_request_work->meta_request = meta_request;
@@ -1019,7 +1011,7 @@ static void s_s3_client_process_work_default(struct aws_s3_client *client) {
             meta_request = NULL;
         }
 
-        aws_mem_release(client->sba_allocator, meta_request_work);
+        aws_mem_release(client->allocator, meta_request_work);
     }
 
     /*******************/
@@ -1314,7 +1306,7 @@ static void s_s3_client_create_connection_for_request_default(
 
     aws_atomic_fetch_add(&client->stats.num_requests_network_io[meta_request->type], 1);
 
-    struct aws_s3_connection *connection = aws_mem_calloc(client->sba_allocator, 1, sizeof(struct aws_s3_connection));
+    struct aws_s3_connection *connection = aws_mem_calloc(client->allocator, 1, sizeof(struct aws_s3_connection));
 
     connection->endpoint = aws_s3_endpoint_acquire(meta_request->endpoint);
     connection->request = request;
@@ -1593,7 +1585,7 @@ void aws_s3_client_notify_connection_finished(
     aws_s3_endpoint_release(connection->endpoint);
     connection->endpoint = NULL;
 
-    aws_mem_release(client->sba_allocator, connection);
+    aws_mem_release(client->allocator, connection);
     connection = NULL;
 
     aws_s3_client_lock_synced_data(client);

diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c
@@ -341,18 +341,16 @@ static void s_s3_prepare_request_payload_callback_and_destroy(
     struct aws_s3_client *client = meta_request->client;
     AWS_PRECONDITION(client);
 
-    struct aws_allocator *sba_allocator = client->sba_allocator;
-    AWS_PRECONDITION(sba_allocator);
+    struct aws_allocator *allocator = client->allocator;
+    AWS_PRECONDITION(allocator);
 
-    /* Acquire a reference to the client pointer just be to safe, so that we know the callback won't trigger a clean up
-     * of the client and cause the sba_allocator to go away. */
     aws_s3_client_acquire(client);
 
     if (payload->callback != NULL) {
         payload->callback(meta_request, payload->request, error_code, payload->user_data);
     }
 
-    aws_mem_release(sba_allocator, payload);
+    aws_mem_release(allocator, payload);
     aws_s3_client_release(client);
 }
 
@@ -388,11 +386,11 @@ static void s_s3_meta_request_schedule_prepare_request_default(
     struct aws_s3_client *client = meta_request->client;
     AWS_PRECONDITION(client);
 
-    struct aws_allocator *sba_allocator = client->sba_allocator;
-    AWS_PRECONDITION(sba_allocator);
+    struct aws_allocator *allocator = client->allocator;
+    AWS_PRECONDITION(allocator);
 
     struct aws_s3_prepare_request_payload *payload =
-        aws_mem_calloc(sba_allocator, 1, sizeof(struct aws_s3_prepare_request_payload));
+        aws_mem_calloc(allocator, 1, sizeof(struct aws_s3_prepare_request_payload));
 
     payload->request = request;
     payload->callback = callback;
@@ -970,7 +968,7 @@ void aws_s3_meta_request_stream_response_body_synced(
     meta_request->synced_data.num_parts_delivery_sent += num_streaming_requests;
 
     struct s3_stream_response_body_payload *payload =
-        aws_mem_calloc(client->sba_allocator, 1, sizeof(struct s3_stream_response_body_payload));
+        aws_mem_calloc(client->allocator, 1, sizeof(struct s3_stream_response_body_payload));
 
     aws_s3_meta_request_acquire(meta_request);
     payload->meta_request = meta_request;
@@ -1046,7 +1044,7 @@ static void s_s3_meta_request_body_streaming_task(struct aws_task *task, void *a
     meta_request->synced_data.num_parts_delivery_succeeded += num_successful;
     aws_s3_meta_request_unlock_synced_data(meta_request);
 
-    aws_mem_release(client->sba_allocator, payload);
+    aws_mem_release(client->allocator, payload);
     payload = NULL;
 
     aws_s3_client_schedule_process_work(client);

diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c
@@ -306,7 +306,7 @@ static int s_test_s3_request_create_destroy(struct aws_allocator *allocator, voi
 
 struct s3_test_body_streaming_user_data {
     struct aws_s3_tester *tester;
-    struct aws_allocator *sba_allocator;
+    struct aws_allocator *allocator;
     uint64_t expected_range_start;
     uint64_t received_body_size;
 };

diff --git a/tests/s3_tester.c b/tests/s3_tester.c
@@ -604,8 +604,6 @@ static void s_s3_mock_client_start_destroy(void *user_data) {
     struct aws_s3_client *client = user_data;
     AWS_ASSERT(client);
 
-    aws_small_block_allocator_destroy(client->sba_allocator);
-
     aws_mem_release(client->allocator, client);
 }
 
@@ -621,8 +619,6 @@ struct aws_s3_client *aws_s3_tester_mock_client_new(struct aws_s3_tester *tester
 
     aws_mutex_init(&mock_client->synced_data.lock);
 
-    mock_client->sba_allocator = aws_small_block_allocator_new(allocator, false);
-
     aws_atomic_init_int(&mock_client->stats.num_requests_in_flight, 0);
 
     for (uint32_t i = 0; i < (uint32_t)AWS_S3_META_REQUEST_TYPE_MAX; ++i) {