From e019473dee9c6317b7e31144bbd00841ccb93131 Mon Sep 17 00:00:00 2001 From: Bert Blommers Date: Thu, 16 Feb 2023 09:52:01 -0100 Subject: [PATCH] S3: Close file-handles of created known FakeKeys explicitly (#5934) --- moto/s3/models.py | 11 +++++ tests/test_s3/test_s3_file_handles.py | 67 ++++++++++++++++++++++++--- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/moto/s3/models.py b/moto/s3/models.py index 6923f6c5e..237975724 100644 --- a/moto/s3/models.py +++ b/moto/s3/models.py @@ -1451,6 +1451,17 @@ class S3Backend(BaseBackend, CloudWatchMetricProvider): def reset(self): # For every key and multipart, Moto opens a TemporaryFile to write the value of those keys # Ensure that these TemporaryFile-objects are closed, and leave no filehandles open + # + # First, check all known buckets/keys + for bucket in self.buckets.values(): + for key in bucket.keys.values(): + if isinstance(key, FakeKey): + key.dispose() + for part in bucket.multiparts.values(): + part.dispose() + # + # Second, go through the list of instances + # It may contain FakeKeys created earlier, which are no longer tracked for mp in FakeMultipart.instances: mp.dispose() for key in FakeKey.instances: diff --git a/tests/test_s3/test_s3_file_handles.py b/tests/test_s3/test_s3_file_handles.py index 216f578cb..b98fb2cef 100644 --- a/tests/test_s3/test_s3_file_handles.py +++ b/tests/test_s3/test_s3_file_handles.py @@ -1,8 +1,9 @@ +import boto3 import copy import gc import warnings from functools import wraps -from moto import settings +from moto import settings, mock_s3 from moto.dynamodb.models import DynamoDBBackend from moto.s3 import models as s3model from moto.s3.responses import S3ResponseInstance @@ -15,9 +16,11 @@ def verify_zero_warnings(f): with warnings.catch_warnings(record=True) as warning_list: warnings.simplefilter("always", ResourceWarning) # add filter resp = f(*args, **kwargs) - # Get the TestClass reference, and reset the S3Backend that we've created as part of that class + # Get the TestClass reference, and reset any S3Backends that we've created as part of that class (class_ref,) = args - class_ref.__dict__["s3"].reset() + for obj in class_ref.__dict__: + if isinstance(obj, s3model.S3Backend): + obj.reset() # Now collect garbage, which will throw any warnings if there are unclosed FileHandles gc.collect() warning_types = [type(warn.message) for warn in warning_list] @@ -41,9 +44,6 @@ class TestS3FileHandleClosures(TestCase): self.s3.create_bucket("versioned-bucket", "us-west-1") self.s3.put_object("my-bucket", "my-key", "x" * 10_000_000) - def tearDown(self) -> None: - self.s3.reset() - @verify_zero_warnings def test_upload_large_file(self): # Verify that we can create an object, as done in the setUp-method @@ -197,6 +197,61 @@ class TestS3FileHandleClosures(TestCase): db.reset() +class TestS3FileHandleClosuresUsingMocks(TestCase): + def setUp(self) -> None: + self.s3 = boto3.client("s3", "us-east-1") + + @verify_zero_warnings + @mock_s3 + def test_use_decorator(self): + self.s3.create_bucket(Bucket="foo") + self.s3.put_object(Bucket="foo", Key="bar", Body="stuff") + + @verify_zero_warnings + @mock_s3 + def test_use_decorator_and_context_mngt(self): + with mock_s3(): + self.s3.create_bucket(Bucket="foo") + self.s3.put_object(Bucket="foo", Key="bar", Body="stuff") + + @verify_zero_warnings + def test_use_multiple_context_managers(self): + with mock_s3(): + self.s3.create_bucket(Bucket="foo") + self.s3.put_object(Bucket="foo", Key="bar", Body="stuff") + + with mock_s3(): + pass + + @verify_zero_warnings + def test_create_multipart(self): + with mock_s3(): + self.s3.create_bucket(Bucket="foo") + self.s3.put_object(Bucket="foo", Key="k1", Body="stuff") + + mp = self.s3.create_multipart_upload(Bucket="foo", Key="key2") + self.s3.upload_part( + Body=b"hello", + PartNumber=1, + Bucket="foo", + Key="key2", + UploadId=mp["UploadId"], + ) + + with mock_s3(): + pass + + @verify_zero_warnings + def test_overwrite_file(self): + with mock_s3(): + self.s3.create_bucket(Bucket="foo") + self.s3.put_object(Bucket="foo", Key="k1", Body="stuff") + self.s3.put_object(Bucket="foo", Key="k1", Body="b" * 10_000_000) + + with mock_s3(): + pass + + def test_verify_key_can_be_copied_after_disposing(): # https://github.com/getmoto/moto/issues/5588 # Multithreaded bug where: