Skip to content

Commit

Permalink
Close container on deletion
Browse files Browse the repository at this point in the history
In case the container is not used within a context and not correctly
closed. The sessions are now correctly closed if the container is picked
up by the garbage collection and deleted.
  • Loading branch information
agoscinski committed Feb 19, 2025
1 parent 6686ad0 commit 078caae
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 2 deletions.
4 changes: 4 additions & 0 deletions disk_objectstore/container.py
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ def __exit__(self, exc_type: Any, exc_value: Any, traceback: Any) -> None:
"""Close the session when exiting the context."""
self.close()

def __del__(self) -> None:
"""Closes all connections on deletion."""
self.close()

def _get_sandbox_folder(self) -> Path:
"""Return the path to the sandbox folder that is used during a new object creation.
Expand Down
44 changes: 42 additions & 2 deletions tests/test_container.py
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,7 @@ def test_sizes(
assert size_info["total_size_loose"] == 0


def test_get_objects_stream_closes(temp_container, generate_random_data):
def test_get_objects_stream_closes(temp_dir, generate_random_data):
"""Test that get_objects_stream_and_meta closes intermediate streams.
I also check that at most one additional file is open at any given time.
Expand All @@ -1155,6 +1155,15 @@ def test_get_objects_stream_closes(temp_container, generate_random_data):
when it goes out of scope - so I add also the test that, inside the loop, at most one more file is open.
The final check seems to always pass even if I forget to do close some file.
"""
# Check if it starts with 0 open files
current_process = psutil.Process()
assert 0 == len(
current_process.open_files()
), "No files should be open at the beginning of test. Maybe some test did not properly close its files."

temp_container = Container(temp_dir)
temp_container.init_container()

data = generate_random_data()
# Store
obj_md5s = _add_objects_loose_loop(temp_container, data)
Expand All @@ -1165,7 +1174,6 @@ def test_get_objects_stream_closes(temp_container, generate_random_data):
# The following checks are still meaningful, I check that if I do it again I don't open more files.
temp_container.get_objects_content(obj_md5s.keys())

current_process = psutil.Process()
start_open_files = len(current_process.open_files())

with temp_container.get_objects_stream_and_meta(
Expand Down Expand Up @@ -1274,6 +1282,38 @@ def test_get_objects_stream_closes(temp_container, generate_random_data):
# Check that at the end nothing is left open
assert len(current_process.open_files()) == start_open_files

# Check if it goes back to 0
temp_container.close()
assert len(current_process.open_files()) == 0


def test_deletion_closes_file_descriptors(temp_dir, generate_random_data):
"""Test if deletion of container closes correctly open file descriptors."""

current_process = psutil.Process()
assert 0 == len(
current_process.open_files()
), "No files should be open at the beginning of test. Maybe some test did not properly close its files."

# Open files
temp_container = Container(temp_dir)
temp_container.init_container(clear=True)
# For Linux to open files it is required to reading from container, on macOS
# the initialization of container is enough
data = generate_random_data()
obj_md5s = _add_objects_loose_loop(temp_container, data)
_ = temp_container.get_objects_content(obj_md5s.keys())

# Checks if initalisation actually opens files
assert 0 < len(
current_process.open_files()
), "No files have been opened during initalisation"

# Checks if deleting the container will close the files
del temp_container

assert 0 == len(current_process.open_files())


def test_get_objects_meta_doesnt_open(
temp_container, generate_random_data
Expand Down

0 comments on commit 078caae

Please sign in to comment.