Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests/test_bmap_helpers.py: avoid build failure from disorderfs #44

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

josch
Copy link

@josch josch commented Jan 19, 2025

When running under disorderfs --shuffle-dirents=yes, the following Python code fails:

with TemporaryDirectory(prefix="testdir_", dir=".") as directory:
    fobj = tempfile.NamedTemporaryFile(
        "r", delete=False, dir=directory
    )
Traceback (most recent call last):
  File "/tmp/disorder/test.py", line 4, in <module>
    with TemporaryDirectory(prefix="testdir_", dir=".") as directory:
  File "/usr/lib/python3.11/tempfile.py", line 1052, in __exit__
    self.cleanup()
  File "/usr/lib/python3.11/tempfile.py", line 1056, in cleanup
    self._rmtree(self.name, ignore_errors=self._ignore_cleanup_errors)
  File "/usr/lib/python3.11/tempfile.py", line 1038, in _rmtree
    _rmtree(name, onerror=onerror)
  File "/usr/lib/python3.11/shutil.py", line 738, in rmtree
    onerror(os.rmdir, path, sys.exc_info())
  File "/usr/lib/python3.11/shutil.py", line 736, in rmtree
    os.rmdir(path, dir_fd=dir_fd)
OSError: [Errno 39] Directory not empty: './testdir_6jhyg1o6'

This might be because the order of deletion is now such that the variable fobj no longer gets cleaned up before rmtree() is called by TemporaryDirectory and thus, the file behind fobj is no longer unlinked early enough.

The problem is fixed by closing fobj (and thus deleting it) before leaving the context manager.

When running under disorderfs --shuffle-dirents=yes, the following Python code
fails:

with TemporaryDirectory(prefix="testdir_", dir=".") as directory:
    fobj = tempfile.NamedTemporaryFile(
        "r", delete=False, dir=directory
    )

Traceback (most recent call last):
  File "/tmp/disorder/test.py", line 4, in <module>
    with TemporaryDirectory(prefix="testdir_", dir=".") as directory:
  File "/usr/lib/python3.11/tempfile.py", line 1052, in __exit__
    self.cleanup()
  File "/usr/lib/python3.11/tempfile.py", line 1056, in cleanup
    self._rmtree(self.name, ignore_errors=self._ignore_cleanup_errors)
  File "/usr/lib/python3.11/tempfile.py", line 1038, in _rmtree
    _rmtree(name, onerror=onerror)
  File "/usr/lib/python3.11/shutil.py", line 738, in rmtree
    onerror(os.rmdir, path, sys.exc_info())
  File "/usr/lib/python3.11/shutil.py", line 736, in rmtree
    os.rmdir(path, dir_fd=dir_fd)
OSError: [Errno 39] Directory not empty: './testdir_6jhyg1o6'

This might be because the order of deletion is now such that the variable fobj
no longer gets cleaned up before rmtree() is called by TemporaryDirectory and
thus, the file behind fobj is no longer unlinked early enough.

The problem is fixed by closing fobj (and thus deleting it) before leaving the
context manager.
@jayaddison
Copy link

From investigating this, the reason that the test fails when using disorderfs is that, by default, FUSE overlay filesystems (of which disorderfs is an instance) don't immediately delete open files completely from the directory they are in; instead they're renamed to a temporary dotfile name, as documented here: http://libfuse.github.io/doxygen/structfuse__config.html#af32ff56fa1131da899756cc352718101

That causes the test to fail, because Python's TemporaryDirectory context-handler quite reasonably thinks that it has removed all the files in the directory, and therefore attempts a delete-only-if-empty operation. This currently fails under disorderfs, because the fobj file is open for reading, and therefore isn't entirely deleted, but only renamed.

Closing the file is a simple fix for this.

Note also: the disorderfs command-line can accept an -o hard_remove option to enable FUSE hard_remove option, causing deletes to apply directly to the underlying filesystem. I've filed a request/suggestion at https://bugs.debian.org/1093768 to enable it by default in disorderfs -- but doing so apparently causes other, potentially more disruptive, side-effects, so there's no guarantee that that behaviour will become the default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants