Removing aws_s3_meta_request_acquire_client (#91)

Author: rccarper

include/aws/s3/private/s3_meta_request_impl.h

@@ -104,6 +104,10 @@ struct aws_s3_meta_request {
 
     struct aws_cached_signing_config_aws *cached_signing_config;
 
+    /* Client that created this meta request which also processes this request.  After the meta request is finished,
+     * this reference is removed. */
+    struct aws_s3_client *client;
+
     /* User data to be passed to each customer specified callback.*/
     void *user_data;
 
@@ -116,10 +120,6 @@ struct aws_s3_meta_request {
     struct {
         struct aws_mutex lock;
 
-        /* Client that created this meta request which also processes this request.  After the meta request is finished,
-         * this reference is removed. */
-        struct aws_s3_client *client;
-
         /* Priority queue for pending streaming requests.  We use a priority queue to keep parts in order so that we
          * can stream them to the caller in order. */
         struct aws_priority_queue pending_body_streaming_requests;
@@ -203,11 +203,6 @@ void aws_s3_meta_request_lock_synced_data(struct aws_s3_meta_request *meta_reque
 AWS_S3_API
 void aws_s3_meta_request_unlock_synced_data(struct aws_s3_meta_request *meta_request);
 
-/* Gets the client reference in the meta request synced_data, acquiring a reference to it if it exists. After calling
- * this function, it is necessary to release that reference. */
-AWS_S3_API
-struct aws_s3_client *aws_s3_meta_request_acquire_client(struct aws_s3_meta_request *meta_request);
-
 /* Called by the client to retrieve the next request and update the meta request's internal state. out_request is
  * optional, and can be NULL if just desiring to update internal state. */
 AWS_S3_API

source/s3_meta_request.c

@@ -81,29 +81,6 @@ void aws_s3_meta_request_unlock_synced_data(struct aws_s3_meta_request *meta_req
     aws_mutex_unlock(&meta_request->synced_data.lock);
 }
 
-struct aws_s3_client *aws_s3_meta_request_acquire_client(struct aws_s3_meta_request *meta_request) {
-    AWS_PRECONDITION(meta_request);
-
-    struct aws_s3_client *client = NULL;
-
-    aws_s3_meta_request_lock_synced_data(meta_request);
-
-    client = meta_request->synced_data.client;
-
-    if (client != NULL) {
-        aws_s3_client_acquire(client);
-    } else {
-        AWS_LOGF_DEBUG(
-            AWS_LS_S3_META_REQUEST,
-            "id=%p Meta request trying to get reference to client but client is null.",
-            (void *)meta_request);
-    }
-
-    aws_s3_meta_request_unlock_synced_data(meta_request);
-
-    return client;
-}
-
 int aws_s3_meta_request_init_base(
     struct aws_allocator *allocator,
     struct aws_s3_client *client,
@@ -159,10 +136,10 @@ int aws_s3_meta_request_init_base(
         sizeof(struct aws_s3_request *),
         s_s3_request_priority_queue_pred);
 
-    /* Client is currently optional to allow spining up a meta_request without a client in a test. */
+    /* Client is currently optional to allow spinning up a meta_request without a client in a test. */
     if (client != NULL) {
         aws_s3_client_acquire(client);
-        meta_request->synced_data.client = client;
+        meta_request->client = client;
     }
 
     meta_request->synced_data.next_streaming_part = 1;
@@ -293,7 +270,7 @@ static void s_s3_meta_request_destroy(void *user_data) {
 
     aws_cached_signing_config_destroy(meta_request->cached_signing_config);
     aws_mutex_clean_up(&meta_request->synced_data.lock);
-    aws_s3_client_release(meta_request->synced_data.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);
@@ -852,7 +829,7 @@ void aws_s3_meta_request_stream_response_body_synced(
         return;
     }
 
-    struct aws_s3_client *client = meta_request->synced_data.client;
+    struct aws_s3_client *client = meta_request->client;
     AWS_ASSERT(client != NULL);
 
     /* Push it into the priority queue. */
@@ -966,7 +943,6 @@ void aws_s3_meta_request_finish_default(struct aws_s3_meta_request *meta_request
     AWS_PRECONDITION(meta_request);
 
     bool already_finished = false;
-    struct aws_s3_client *client = NULL;
     struct aws_linked_list release_request_list;
     aws_linked_list_init(&release_request_list);
 
@@ -982,10 +958,6 @@ void aws_s3_meta_request_finish_default(struct aws_s3_meta_request *meta_request
 
     meta_request->synced_data.state = AWS_S3_META_REQUEST_STATE_FINISHED;
 
-    /* Get rid of our client reference. Release it outside of the lock. */
-    client = meta_request->synced_data.client;
-    meta_request->synced_data.client = NULL;
-
     /* Clean out the pending-stream-to-caller priority queue*/
     while (aws_priority_queue_size(&meta_request->synced_data.pending_body_streaming_requests) > 0) {
         struct aws_s3_request *request = NULL;
@@ -1012,7 +984,8 @@ void aws_s3_meta_request_finish_default(struct aws_s3_meta_request *meta_request
         aws_s3_request_release(release_request);
     }
 
-    aws_s3_client_release(client);
+    aws_s3_client_release(meta_request->client);
+    meta_request->client = NULL;
 
     AWS_LOGF_DEBUG(
         AWS_LS_S3_META_REQUEST,

source/s3_request.c

@@ -39,7 +39,7 @@ void aws_s3_request_setup_send_data(struct aws_s3_request *request, struct aws_h
     aws_http_message_acquire(message);
 }
 
-void s_s3_request_clean_up_send_data_message(struct aws_s3_request *request) {
+static void s_s3_request_clean_up_send_data_message(struct aws_s3_request *request) {
     AWS_PRECONDITION(request);
 
     struct aws_http_message *message = request->send_data.message;
@@ -98,12 +98,10 @@ static void s_s3_request_destroy(void *user_data) {
     struct aws_s3_meta_request *meta_request = request->meta_request;
 
     if (meta_request != NULL) {
-        struct aws_s3_client *client = aws_s3_meta_request_acquire_client(meta_request);
+        struct aws_s3_client *client = meta_request->client;
 
         if (client != NULL) {
             aws_s3_client_notify_request_destroyed(client, request);
-            aws_s3_client_release(client);
-            client = NULL;
         }
     }
 

tests/s3_retry_tests.c

@@ -297,15 +297,12 @@ static void s_s3_meta_request_send_request_finish_fail_first(
     struct aws_http_stream *stream,
     int error_code) {
 
-    struct aws_s3_client *client = aws_s3_meta_request_acquire_client(vip_connection->request->meta_request);
+    struct aws_s3_client *client = vip_connection->request->meta_request->client;
     AWS_ASSERT(client != NULL);
 
     struct aws_s3_tester *tester = client->shutdown_callback_user_data;
     AWS_ASSERT(tester != NULL);
 
-    aws_s3_client_release(client);
-    client = NULL;
-
     if (aws_s3_tester_inc_counter2(tester) == 1) {
         AWS_ASSERT(vip_connection->request->send_data.response_status == 404);
 
@@ -376,12 +373,9 @@ static void s_finished_request_remove_upload_id(
         aws_byte_buf_reset(&request->send_data.response_body, false);
     }
 
-    struct aws_s3_client *client = aws_s3_meta_request_acquire_client(meta_request);
+    struct aws_s3_client *client = meta_request->client;
     struct aws_s3_tester *tester = client->shutdown_callback_user_data;
 
-    aws_s3_client_release(client);
-    client = NULL;
-
     struct aws_s3_meta_request_vtable *original_meta_request_vtable =
         aws_s3_tester_get_meta_request_vtable_patch(tester, 0)->original_vtable;