Fix crash when aws_s3_meta_request has function called on it after co…

…mpletion (#217)

**Issue**:
If a function was called on a meta-request after it completed, this would lead to a crash (this affected `aws_s3_meta_request_increment_read_window()`, `aws_s3_meta_request_cancel()`, and `aws_s3_meta_request_pause()`)

Since these functions might be called from different threads than the thread that completes the meta-request, it would be nearly impossible for a customer to avoid these crashes.

**Description of changes:**
The reason for the crash was that the `meta_request->client` pointer would get cleared when the meta-request completed.

To fix this bug, we stop clearing this pointer early, and keep`meta_request->client` pointer valid for the lifetime of the meta-request.

We evaluated doing something much more complex (checking state and toying with reference counts before scheduling work on the client) but it seemed finicky and error-prone.

samples/s3/cli_progress_bar.c

@@ -45,8 +45,7 @@ struct progress_listener_group *progress_listener_group_new(struct aws_allocator
     aws_mutex_init(&group->mutex);
     group->render_sink = stdout;
     aws_array_list_init_dynamic(&group->listeners, allocator, 16, sizeof(struct progress_listener *));
-    struct aws_thread_options options;
-    AWS_ZERO_STRUCT(options);
+    struct aws_thread_options options = *aws_default_thread_options();
 
     group->scheduler = aws_thread_scheduler_new(allocator, &options);
     return group;

source/s3_meta_request.c

@@ -415,7 +415,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);
-    /* endpoint and client should have already been released and set NULL by the meta request finish call.
+    /* 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);
@@ -1479,9 +1479,6 @@ void aws_s3_meta_request_finish_default(struct aws_s3_meta_request *meta_request
     aws_s3_endpoint_release(meta_request->endpoint);
     meta_request->endpoint = NULL;
 
-    aws_s3_client_release(meta_request->client);
-    meta_request->client = NULL;
-
     meta_request->io_event_loop = NULL;
 }
 

tests/s3_data_plane_tests.c

@@ -1521,6 +1521,10 @@ static int s_test_s3_get_object_backpressure_helper(
 
     ASSERT_SUCCESS(aws_s3_tester_validate_get_object_results(&meta_request_test_results, 0));
 
+    /* Regression test:
+     * 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;