diff --git a/include/aws/s3/s3_client.h b/include/aws/s3/s3_client.h index 8b36eaf7b..f061f17da 100644 --- a/include/aws/s3/s3_client.h +++ b/include/aws/s3/s3_client.h @@ -453,11 +453,23 @@ struct aws_s3_client *aws_s3_client_new( struct aws_allocator *allocator, const struct aws_s3_client_config *client_config); +/** + * Add a reference, keeping this object alive. + * The reference must be released when you are done with it, or it's memory will never be cleaned up. + * You must not pass in NULL. + * Always returns the same pointer that was passed in. + */ AWS_S3_API -void aws_s3_client_acquire(struct aws_s3_client *client); +struct aws_s3_client *aws_s3_client_acquire(struct aws_s3_client *client); +/** + * Release a reference. + * When the reference count drops to 0, this object will be cleaned up. + * It's OK to pass in NULL (nothing happens). + * Always returns NULL. + */ AWS_S3_API -void aws_s3_client_release(struct aws_s3_client *client); +struct aws_s3_client *aws_s3_client_release(struct aws_s3_client *client); AWS_S3_API struct aws_s3_meta_request *aws_s3_client_make_meta_request( @@ -505,11 +517,23 @@ void aws_s3_meta_request_cancel(struct aws_s3_meta_request *meta_request); AWS_S3_API int aws_s3_meta_request_pause(struct aws_s3_meta_request *meta_request, struct aws_string **out_resume_token); +/** + * Add a reference, keeping this object alive. + * The reference must be released when you are done with it, or it's memory will never be cleaned up. + * You must not pass in NULL. + * Always returns the same pointer that was passed in. + */ AWS_S3_API -void aws_s3_meta_request_acquire(struct aws_s3_meta_request *meta_request); +struct aws_s3_meta_request *aws_s3_meta_request_acquire(struct aws_s3_meta_request *meta_request); +/** + * Release a reference. + * When the reference count drops to 0, this object will be cleaned up. + * It's OK to pass in NULL (nothing happens). + * Always returns NULL. + */ AWS_S3_API -void aws_s3_meta_request_release(struct aws_s3_meta_request *meta_request); +struct aws_s3_meta_request *aws_s3_meta_request_release(struct aws_s3_meta_request *meta_request); AWS_S3_API void aws_s3_init_default_signing_config( diff --git a/source/s3_client.c b/source/s3_client.c index 2a7fb1d85..43ad6f974 100644 --- a/source/s3_client.c +++ b/source/s3_client.c @@ -476,18 +476,19 @@ struct aws_s3_client *aws_s3_client_new( return NULL; } -void aws_s3_client_acquire(struct aws_s3_client *client) { +struct aws_s3_client *aws_s3_client_acquire(struct aws_s3_client *client) { AWS_PRECONDITION(client); aws_ref_count_acquire(&client->ref_count); + return client; } -void aws_s3_client_release(struct aws_s3_client *client) { - if (client == NULL) { - return; +struct aws_s3_client *aws_s3_client_release(struct aws_s3_client *client) { + if (client != NULL) { + aws_ref_count_release(&client->ref_count); } - aws_ref_count_release(&client->ref_count); + return NULL; } static void s_s3_client_start_destroy(void *user_data) { @@ -830,8 +831,7 @@ struct aws_s3_meta_request *aws_s3_client_make_meta_request( aws_last_error(), aws_error_str(aws_last_error())); - aws_s3_meta_request_release(meta_request); - meta_request = NULL; + meta_request = aws_s3_meta_request_release(meta_request); } else { AWS_LOGF_INFO(AWS_LS_S3_CLIENT, "id=%p: Created meta request %p", (void *)client, (void *)meta_request); } @@ -1165,8 +1165,7 @@ static void s_s3_client_process_work_default(struct aws_s3_client *client) { meta_request->client_process_work_threaded_data.scheduled = true; } else { - aws_s3_meta_request_release(meta_request); - meta_request = NULL; + meta_request = aws_s3_meta_request_release(meta_request); } aws_mem_release(client->allocator, meta_request_work); diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 5b54c17f5..42c1f9111 100644 --- a/source/s3_meta_request.c +++ b/source/s3_meta_request.c @@ -368,18 +368,19 @@ bool aws_s3_meta_request_has_finish_result_synced(struct aws_s3_meta_request *me return true; } -void aws_s3_meta_request_acquire(struct aws_s3_meta_request *meta_request) { +struct aws_s3_meta_request *aws_s3_meta_request_acquire(struct aws_s3_meta_request *meta_request) { AWS_PRECONDITION(meta_request); aws_ref_count_acquire(&meta_request->ref_count); + return meta_request; } -void aws_s3_meta_request_release(struct aws_s3_meta_request *meta_request) { - if (meta_request == NULL) { - return; +struct aws_s3_meta_request *aws_s3_meta_request_release(struct aws_s3_meta_request *meta_request) { + if (meta_request != NULL) { + aws_ref_count_release(&meta_request->ref_count); } - aws_ref_count_release(&meta_request->ref_count); + return NULL; } static void s_s3_meta_request_destroy(void *user_data) { @@ -402,7 +403,7 @@ static void s_s3_meta_request_destroy(void *user_data) { /* endpoint should have already been released and set NULL by the meta request finish call. * But call release() again, just in case we're tearing down a half-initialized meta request */ aws_s3_endpoint_release(meta_request->endpoint); - aws_s3_client_release(meta_request->client); + meta_request->client = aws_s3_client_release(meta_request->client); AWS_ASSERT(aws_priority_queue_size(&meta_request->synced_data.pending_body_streaming_requests) == 0); aws_priority_queue_clean_up(&meta_request->synced_data.pending_body_streaming_requests); @@ -481,14 +482,12 @@ static void s_s3_prepare_request_payload_callback_and_destroy( struct aws_s3_meta_request *meta_request = payload->request->meta_request; AWS_PRECONDITION(meta_request); - struct aws_s3_client *client = meta_request->client; - AWS_PRECONDITION(client); + AWS_PRECONDITION(meta_request->client); + struct aws_s3_client *client = aws_s3_client_acquire(meta_request->client); struct aws_allocator *allocator = client->allocator; AWS_PRECONDITION(allocator); - aws_s3_client_acquire(client); - if (payload->callback != NULL) { payload->callback(meta_request, payload->request, error_code, payload->user_data); } diff --git a/source/s3_paginator.c b/source/s3_paginator.c index 1b5522b04..2c8506f07 100644 --- a/source/s3_paginator.c +++ b/source/s3_paginator.c @@ -120,7 +120,7 @@ struct aws_s3_paginator *aws_s3_initiate_paginator( struct aws_s3_paginator *paginator = aws_mem_calloc(allocator, 1, sizeof(struct aws_s3_paginator)); paginator->allocator = allocator; - paginator->client = params->client; + paginator->client = aws_s3_client_acquire(params->client); paginator->operation = params->operation; paginator->on_page_finished = params->on_page_finished_fn; paginator->user_data = params->user_data; @@ -128,7 +128,6 @@ struct aws_s3_paginator *aws_s3_initiate_paginator( paginator->bucket_name = aws_string_new_from_cursor(allocator, ¶ms->bucket_name); paginator->endpoint = aws_string_new_from_cursor(allocator, ¶ms->endpoint); - aws_s3_client_acquire(params->client); aws_s3_paginated_operation_acquire(params->operation); aws_byte_buf_init(&paginator->result_body, allocator, s_dynamic_body_initial_buf_size); diff --git a/source/s3_request.c b/source/s3_request.c index 35a99b847..eb3c888bb 100644 --- a/source/s3_request.c +++ b/source/s3_request.c @@ -18,8 +18,7 @@ struct aws_s3_request *aws_s3_request_new( aws_ref_count_init(&request->ref_count, request, (aws_simple_completion_callback *)s_s3_request_destroy); request->allocator = meta_request->allocator; - request->meta_request = meta_request; - aws_s3_meta_request_acquire(meta_request); + request->meta_request = aws_s3_meta_request_acquire(meta_request); request->request_tag = request_tag; request->part_number = part_number; diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c index fc68fa7f6..ed4ef27a7 100644 --- a/tests/s3_data_plane_tests.c +++ b/tests/s3_data_plane_tests.c @@ -43,8 +43,7 @@ static int s_test_s3_client_create_destroy(struct aws_allocator *allocator, void ASSERT_TRUE(client != NULL); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -326,8 +325,7 @@ static int s_test_s3_client_get_max_active_connections(struct aws_allocator *all } for (size_t i = 0; i < AWS_S3_META_REQUEST_TYPE_MAX; ++i) { - aws_s3_meta_request_release(mock_meta_requests[i]); - mock_meta_requests[i] = NULL; + mock_meta_requests[i] = aws_s3_meta_request_release(mock_meta_requests[i]); } aws_s3_client_release(mock_client); @@ -452,8 +450,7 @@ static int s_test_s3_meta_request_body_streaming(struct aws_allocator *allocator ASSERT_TRUE(meta_request != NULL); struct aws_event_loop_group *event_loop_group = aws_event_loop_group_new_default(allocator, 0, NULL); - aws_s3_client_acquire(mock_client); - meta_request->client = mock_client; + meta_request->client = aws_s3_client_acquire(mock_client); meta_request->user_data = &body_streaming_user_data; *((size_t *)&meta_request->part_size) = request_response_body_size; meta_request->body_callback = s_s3_meta_request_test_body_streaming_callback; @@ -1060,8 +1057,7 @@ static int s_test_s3_get_object_helper( aws_tls_ctx_options_clean_up(&tls_context_options); #endif - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1226,8 +1222,7 @@ static int s_test_s3_get_object_less_than_part_size(struct aws_allocator *alloca ASSERT_SUCCESS(aws_s3_tester_send_get_object_meta_request( &tester, client, g_s3_path_get_object_test_1MB, AWS_S3_TESTER_SEND_META_REQUEST_EXPECT_SUCCESS, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1331,8 +1326,7 @@ static int s_test_s3_get_object_multiple(struct aws_allocator *allocator, void * aws_s3_tester_unlock_synced_data(&tester); for (size_t i = 0; i < num_meta_requests; ++i) { - aws_s3_meta_request_release(meta_requests[i]); - meta_requests[i] = NULL; + meta_requests[i] = aws_s3_meta_request_release(meta_requests[i]); } aws_s3_tester_wait_for_meta_request_shutdown(&tester); @@ -1345,8 +1339,7 @@ static int s_test_s3_get_object_multiple(struct aws_allocator *allocator, void * aws_string_destroy(host_name); host_name = NULL; - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1522,8 +1515,7 @@ static int s_test_s3_get_object_backpressure_helper( * Ensure that it's safe to call increment-window even after the meta-request has finished */ aws_s3_meta_request_increment_read_window(meta_request, 1024); - aws_s3_meta_request_release(meta_request); - meta_request = NULL; + meta_request = aws_s3_meta_request_release(meta_request); aws_s3_tester_wait_for_meta_request_shutdown(&tester); @@ -1535,8 +1527,7 @@ static int s_test_s3_get_object_backpressure_helper( aws_string_destroy(host_name); host_name = NULL; - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1640,8 +1631,7 @@ static int s_test_s3_put_object_helper( aws_tls_ctx_options_clean_up(&tls_context_options); #endif - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1751,8 +1741,7 @@ static int s_test_s3_put_object_multiple(struct aws_allocator *allocator, void * aws_s3_tester_unlock_synced_data(&tester); for (size_t i = 0; i < num_meta_requests; ++i) { - aws_s3_meta_request_release(meta_requests[i]); - meta_requests[i] = NULL; + meta_requests[i] = aws_s3_meta_request_release(meta_requests[i]); } aws_s3_tester_wait_for_meta_request_shutdown(&tester); @@ -1771,8 +1760,7 @@ static int s_test_s3_put_object_multiple(struct aws_allocator *allocator, void * aws_string_destroy(host_name); host_name = NULL; - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1800,8 +1788,7 @@ static int s_test_s3_put_object_less_than_part_size(struct aws_allocator *alloca ASSERT_SUCCESS(aws_s3_tester_send_put_object_meta_request( &tester, client, 1, AWS_S3_TESTER_SEND_META_REQUEST_EXPECT_SUCCESS, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1828,8 +1815,7 @@ static int s_test_s3_put_object_empty_object(struct aws_allocator *allocator, vo ASSERT_SUCCESS(aws_s3_tester_send_put_object_meta_request( &tester, client, 0, AWS_S3_TESTER_SEND_META_REQUEST_EXPECT_SUCCESS, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1861,8 +1847,7 @@ static int s_test_s3_put_object_sse_kms(struct aws_allocator *allocator, void *c AWS_S3_TESTER_SEND_META_REQUEST_EXPECT_SUCCESS | AWS_S3_TESTER_SEND_META_REQUEST_SSE_KMS, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1894,8 +1879,7 @@ static int s_test_s3_put_object_sse_kms_multipart(struct aws_allocator *allocato AWS_S3_TESTER_SEND_META_REQUEST_EXPECT_SUCCESS | AWS_S3_TESTER_SEND_META_REQUEST_SSE_KMS, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1927,8 +1911,7 @@ static int s_test_s3_put_object_sse_aes256(struct aws_allocator *allocator, void AWS_S3_TESTER_SEND_META_REQUEST_EXPECT_SUCCESS | AWS_S3_TESTER_SEND_META_REQUEST_SSE_AES256, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -1960,8 +1943,7 @@ static int s_test_s3_put_object_sse_aes256_multipart(struct aws_allocator *alloc AWS_S3_TESTER_SEND_META_REQUEST_EXPECT_SUCCESS | AWS_S3_TESTER_SEND_META_REQUEST_SSE_AES256, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -2001,8 +1983,7 @@ static int s_test_s3_put_object_sse_c_aes256_multipart(struct aws_allocator *all }; ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -2045,8 +2026,7 @@ static int s_test_s3_put_object_sse_c_aes256_multipart_with_checksum(struct aws_ }; ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -2082,8 +2062,7 @@ static int s_test_s3_put_object_content_md5_helper( ASSERT_SUCCESS(aws_s3_tester_send_put_object_meta_request(&tester, client, 10, flags, NULL)); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -3018,8 +2997,7 @@ static int s_test_s3_meta_request_default(struct aws_allocator *allocator, void ASSERT_SUCCESS(aws_s3_tester_validate_get_object_results(&meta_request_test_results, 0)); - aws_s3_meta_request_release(meta_request); - meta_request = NULL; + meta_request = aws_s3_meta_request_release(meta_request); aws_s3_tester_wait_for_meta_request_shutdown(&tester); @@ -3031,8 +3009,7 @@ static int s_test_s3_meta_request_default(struct aws_allocator *allocator, void aws_string_destroy(host_name); host_name = NULL; - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -3093,8 +3070,7 @@ static int s_test_s3_error_missing_file(struct aws_allocator *allocator, void *c ASSERT_TRUE(meta_request_test_results.error_response_headers != NULL); - aws_s3_meta_request_release(meta_request); - meta_request = NULL; + meta_request = aws_s3_meta_request_release(meta_request); aws_s3_tester_wait_for_meta_request_shutdown(&tester); aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); @@ -3105,8 +3081,7 @@ static int s_test_s3_error_missing_file(struct aws_allocator *allocator, void *c aws_string_destroy(host_name); host_name = NULL; - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -3232,8 +3207,7 @@ static int s_test_s3_bad_endpoint(struct aws_allocator *allocator, void *ctx) { aws_http_message_release(message); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -3745,8 +3719,7 @@ static int s_test_s3_put_object_clamp_part_size(struct aws_allocator *allocator, aws_s3_meta_request_test_results_clean_up(&test_results); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -4504,8 +4477,7 @@ static int s_test_s3_copy_object_from_x_amz_copy_source( aws_s3_meta_request_release(meta_request); aws_mutex_clean_up(&test_data.mutex); aws_http_message_destroy(message); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -4691,8 +4663,7 @@ static int s_s3_get_object_mrap_helper(struct aws_allocator *allocator, bool mul ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, &meta_request_test_results)); aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -4753,8 +4724,7 @@ static int s_s3_put_object_mrap_helper(struct aws_allocator *allocator, bool mul ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, &meta_request_test_results)); aws_s3_meta_request_test_results_clean_up(&meta_request_test_results); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); @@ -5062,8 +5032,7 @@ static int s_test_s3_put_pause_resume_helper( aws_s3_meta_request_test_results_clean_up(&results); } - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); return 0; } diff --git a/tests/s3_retry_tests.c b/tests/s3_retry_tests.c index 085af6dfa..238bab130 100644 --- a/tests/s3_retry_tests.c +++ b/tests/s3_retry_tests.c @@ -198,8 +198,7 @@ static int s_test_s3_meta_request_fail_prepare_request(struct aws_allocator *all aws_s3_tester_wait_for_counters(&tester); - aws_s3_client_release(client); - client = NULL; + client = aws_s3_client_release(client); aws_s3_tester_clean_up(&tester); diff --git a/tests/s3_tester.c b/tests/s3_tester.c index 5cfea80d0..d2383a099 100644 --- a/tests/s3_tester.c +++ b/tests/s3_tester.c @@ -1413,8 +1413,7 @@ int aws_s3_tester_send_meta_request_with_options( if (meta_request != NULL) { out_results->part_size = meta_request->part_size; - aws_s3_meta_request_release(meta_request); - meta_request = NULL; + meta_request = aws_s3_meta_request_release(meta_request); if (!options->dont_wait_for_shutdown) { aws_s3_tester_wait_for_meta_request_shutdown(tester);