Fix bug when S3Request is GC'd before download completes (#506)

**Issue:**
I discovered that, if you drop the reference to an S3Request before the operation completes and it's downloading using `recv_filepath`, then everything looks like it succeeds but you end up with a 0 byte file on disk.

**Description of changes:**
Ensure the file isn't closed until the operation is complete.

Most of our code was careful to keep things alive until the operation was complete. But for whatever reason we put the file-closing code in the wrong place.

Author: graebm

source/s3_meta_request.c

@@ -44,6 +44,9 @@ struct aws_s3_meta_request *aws_py_get_s3_meta_request(PyObject *meta_request) {
 }
 
 static void s_destroy(struct s3_meta_request_binding *meta_request) {
+    if (meta_request->recv_file) {
+        fclose(meta_request->recv_file);
+    }
     if (meta_request->copied_message) {
         aws_http_message_release(meta_request->copied_message);
     }
@@ -297,14 +300,13 @@ static void s_s3_request_on_finish(
     /*************** GIL RELEASE ***************/
 }
 
-/* Invoked when the python object get cleaned up */
+/* Invoked when S3Request._binding gets cleaned up.
+ * DO NOT destroy the C binding struct or anything inside it yet.
+ * The user might have let S3Request get GC'd,
+ * but the s3_meta_request_binding* must outlive the native aws_s3_meta_request* */
 static void s_s3_meta_request_capsule_destructor(PyObject *capsule) {
     struct s3_meta_request_binding *meta_request = PyCapsule_GetPointer(capsule, s_capsule_name_s3_meta_request);
 
-    if (meta_request->recv_file) {
-        fclose(meta_request->recv_file);
-        meta_request->recv_file = NULL;
-    }
     if (meta_request->native) {
         aws_s3_meta_request_release(meta_request->native);
     } else {

test/test_s3.py

@@ -270,6 +270,13 @@ def test_get_object_file_object(self):
                 on_headers=self._on_request_headers,
                 on_progress=self._on_progress)
             finished_future = s3_request.finished_future
+
+            # Regression test: Let S3Request get GC'd early.
+            # The download should continue without problems.
+            # We once had a bug where the file would get closed too early:
+            # https://github.com/awslabs/aws-crt-python/pull/506
+            del s3_request
+
             finished_future.result(self.timeout)
 
             # Result check