diff --git a/source/s3_meta_request.c b/source/s3_meta_request.c index 13ace451a..e471c64e5 100644 --- a/source/s3_meta_request.c +++ b/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 { diff --git a/test/test_s3.py b/test/test_s3.py index 5de4a8cd6..0d1a0c0ef 100644 --- a/test/test_s3.py +++ b/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