Fix shutdown_callback or returning NULL contract for meta_request (#440)

Co-authored-by: Michael Graeb <graebm@amazon.com>
awslabs · Jun 20, 2024 · a549581 · a549581
1 parent b93e534
commit a549581
Show file tree

Hide file tree

Showing 6 changed files with 12 additions and 4 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -6,7 +6,7 @@ on:
       - 'main'
 
 env:
-  BUILDER_VERSION: v0.9.55
+  BUILDER_VERSION: v0.9.59
   BUILDER_SOURCE: releases
   BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net
   PACKAGE_NAME: aws-c-s3

diff --git a/source/s3_client.c b/source/s3_client.c
@@ -1090,6 +1090,11 @@ struct aws_s3_meta_request *aws_s3_client_make_meta_request(
         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);
+        /**
+         * shutdown_callback must be the last thing that gets set on the meta_request so that we don’t return NULL and
+         * trigger the shutdown_callback.
+         */
+        meta_request->shutdown_callback = options->shutdown_callback;
     }
 
     return meta_request;

diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c
@@ -290,7 +290,6 @@ int aws_s3_meta_request_init_base(
 
     meta_request->meta_request_level_running_response_sum = NULL;
     meta_request->user_data = options->user_data;
-    meta_request->shutdown_callback = options->shutdown_callback;
     meta_request->progress_callback = options->progress_callback;
     meta_request->telemetry_callback = options->telemetry_callback;
     meta_request->upload_review_callback = options->upload_review_callback;

diff --git a/tests/s3_data_plane_tests.c b/tests/s3_data_plane_tests.c
@@ -5960,6 +5960,7 @@ static int s_test_s3_invalid_start_range_greator_than_end_range(struct aws_alloc
 
     ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &options, &results));
     ASSERT_INT_EQUALS(results.finished_error_code, AWS_ERROR_S3_INVALID_RANGE_HEADER);
+    ASSERT_INT_EQUALS(0, tester.synced_data.meta_request_finish_count);
 
     aws_s3_meta_request_test_results_clean_up(&results);
 

diff --git a/tests/s3_mock_server_tests.c b/tests/s3_mock_server_tests.c
@@ -930,7 +930,7 @@ TEST_CASE(endpoint_override_mock_server) {
         .client = client,
         .put_options =
             {
-                .object_size_mb = 5, /* Make sure we have exactly 4 parts */
+                .object_size_mb = 5,
                 .object_path_override = object_path,
             },
         .mock_server = true,
@@ -956,6 +956,7 @@ TEST_CASE(endpoint_override_mock_server) {
     put_options.message = message;
     put_options.validate_type = AWS_S3_TESTER_VALIDATE_TYPE_EXPECT_FAILURE;
     ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, NULL));
+    ASSERT_INT_EQUALS(2, tester.synced_data.meta_request_shutdown_count);
 
     /* Clean up */
     aws_http_message_destroy(message);

diff --git a/tests/s3_tester.c b/tests/s3_tester.c
@@ -1686,9 +1686,11 @@ int aws_s3_tester_send_meta_request_with_options(
         if (options->meta_request_type == AWS_S3_META_REQUEST_TYPE_PUT_OBJECT) {
             /* Figure out how much is being uploaded from pre-existing message */
             struct aws_input_stream *mystery_stream = aws_http_message_get_body_stream(meta_request_options.message);
+            int64_t mystery_stream_length = 0;
             if (mystery_stream != NULL) {
-                ASSERT_SUCCESS(aws_input_stream_get_length(mystery_stream, (int64_t *)&upload_size_bytes));
+                ASSERT_SUCCESS(aws_input_stream_get_length(mystery_stream, (int64_t *)&mystery_stream_length));
             }
+            upload_size_bytes = (size_t)mystery_stream_length;
         }
     }