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

pyfakefs conflicts with fasteners on file locking: bad file descriptor #645

Closed
Wenzel opened this issue Nov 1, 2021 · 8 comments
Closed

Comments

@Wenzel
Copy link

Wenzel commented Nov 1, 2021

Describe the bug
While working on my library based on apache-libcloud, I used the local provider to use the filesystem as an object storage.

While I wanted to mix this with pyfakefs in my unit test, this resulted in a bad file descriptor error in the test teadown, with a potential conflict between the fasteners library and pyfakefs logic.

How To Reproduce
Surprisingly, I found this bug only when running my test suite on Github Actions.
Running it on my laptop works fine, and I have no clue at this point with the Python environment would be different in Github Actions.

I build a small repository for you to test this issue and have a 100% repro:
https://github.com/Wenzel/test_bug_pyfaefs_github_actions/tree/test

(Please note the test branch where from which I have opened a pull request to trigger Github Actions and have a repro)

I created 2 tests:

  • test_fake_fs
  • test_no_fake_fs

Each one will use a fixture to create a libcloud object storage driver based on the local provider (filesystem), but one of them also uses pyfakefs, instead of the real filesystem.

When the fake filesystem is used, the teardown fails, as you can see below. (But only on Github Actions ❓ ❗ )

This is the stacktrace that I get on Github Actions:

==================================== ERRORS ====================================
______________________ ERROR at teardown of test_fake_fs _______________________

self = <fasteners.process_lock._FcntlLock object at 0x7fd702951dc0>
blocking = True, watch = <fasteners._utils.StopWatch object at 0x7fd702951f40>

    def _try_acquire(self, blocking, watch):
        try:
>           self.trylock()

../../../.cache/pypoetry/virtualenvs/test-bad-file-descriptor-gyJ7bpKD-py3.8/lib/python3.8/site-packages/fasteners/process_lock.py:78: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <fasteners.process_lock._FcntlLock object at 0x7fd702951dc0>

    def trylock(self):
>       self._trylock(self.lockfile)

../../../.cache/pypoetry/virtualenvs/test-bad-file-descriptor-gyJ7bpKD-py3.8/lib/python3.8/site-packages/fasteners/process_lock.py:196: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

lockfile = <pyfakefs.fake_filesystem.FakeFileWrapper object at 0x7fd7029670a0>

    @staticmethod
    def _trylock(lockfile):
>       fcntl.lockf(lockfile, fcntl.LOCK_EX | fcntl.LOCK_NB)
E       OSError: [Errno 9] Bad file descriptor

../../../.cache/pypoetry/virtualenvs/test-bad-file-descriptor-gyJ7bpKD-py3.8/lib/python3.8/site-packages/fasteners/process_lock.py:432: OSError

During handling of the above exception, another exception occurred:

fs = <pyfakefs.fake_filesystem.FakeFilesystem object at 0x7fd701d1d610>
tmp_dir = PosixPath('/tmp/tmprt34tghc')

    @pytest.fixture
    def libcloud_driver_fake_fs(fs, tmp_dir):
        """Initializes libcloud based on the fake filesystem PyFakeFS"""
        with libcloud_driver(tmp_dir) as driver:
>           yield driver

tests/test_01.py:32: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/opt/hostedtoolcache/Python/3.8.12/x64/lib/python3.8/contextlib.py:120: in __exit__
    next(self.gen)
tests/test_01.py:25: in libcloud_driver
    driver.delete_container(cont)
../../../.cache/pypoetry/virtualenvs/test-bad-file-descriptor-gyJ7bpKD-py3.8/lib/python3.8/site-packages/libcloud/storage/drivers/local.py:664: in delete_container
    with LockLocalStorage(path):
../../../.cache/pypoetry/virtualenvs/test-bad-file-descriptor-gyJ7bpKD-py3.8/lib/python3.8/site-packages/libcloud/storage/drivers/local.py:89: in __enter__
    success = self.ipc_lock.acquire(blocking=True,
../../../.cache/pypoetry/virtualenvs/test-bad-file-descriptor-gyJ7bpKD-py3.8/lib/python3.8/site-packages/fasteners/process_lock.py:140: in acquire
    gotten = r(self._try_acquire, blocking, watch)
../../../.cache/pypoetry/virtualenvs/test-bad-file-descriptor-gyJ7bpKD-py3.8/lib/python3.8/site-packages/fasteners/_utils.py:124: in __call__
    return fn(*args, **kwargs)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <fasteners.process_lock._FcntlLock object at 0x7fd702951dc0>
blocking = True, watch = <fasteners._utils.StopWatch object at 0x7fd702951f40>

    def _try_acquire(self, blocking, watch):
        try:
            self.trylock()
        except IOError as e:
            if e.errno in (errno.EACCES, errno.EAGAIN):
                if not blocking or watch.expired():
                    return False
                else:
                    raise _utils.RetryAgain()
            else:
>               raise threading.ThreadError("Unable to acquire lock on"
                                            " `%(path)s` due to"
                                            " %(exception)s" %
                                            {
                                                'path': self.path,
                                                'exception': e,
                                            })
E               RuntimeError: Unable to acquire lock on `b'/tmp/3abf221e48c97e8e7239ce5aceb53aeb2296dd948939cb89e08856bede3265c6.lock'` due to [Errno 9] Bad file descriptor

../../../.cache/pypoetry/virtualenvs/test-bad-file-descriptor-gyJ7bpKD-py3.8/lib/python3.8/site-packages/fasteners/process_lock.py:86: RuntimeError
=========================== short test summary info ============================
ERROR tests/test_01.py::test_fake_fs - RuntimeError: Unable to acquire lock o...
========================== 2 passed, 1 error in 0.28s ==========================

Your environment

My local environment

Linux-5.4.0-88-generic-x86_64-with-Ubuntu-20.04-focal
('Python', '2.7.18 (default, Mar  8 2021, 13:02:45) \n[GCC 9.3.0]')
pyfakefs 4.5.1

I hope you have enough information at this point to better understand / investigate the bug.

Feel free to ask for more context !

Thank you for maintaining pyfakefs 🙂

@mrbean-bremen
Copy link
Member

Thank you for the report! This does not look like an easy one. From your callstack I can see that fasteners use fcntl functions, which are not mocked. These work on file descriptors, and the descriptor it gets is from the fake filesystem, so it fails. I guess on your local file system it works because the descriptor is also valid in the local file system (this depends on the number of open files, which may differ in the CI environment), and the locking succeeds, even as it actually locks the wrong file.
The easiest fix would probably to mock these functions away, so that they do nothing, but it would probably be better to emulate at least part of the functionality. fcntl is a Posix-only relatively low-level C module, I'm not sure what patching this would involve, but I will have a look. This will take some time, though, and I cannot guarantee a fix...

@Wenzel
Copy link
Author

Wenzel commented Nov 1, 2021

The easiest fix would probably to mock these functions away, so that they do nothing, but it would probably be better to emulate at least part of the functionality. fcntl is a Posix-only relatively low-level C module, I'm not sure what patching this would involve, but I will have a look.

Thank you for the prompt response !

On my side, I managed to comment some code in my test suite, and the bug doesn't trigger anymore, so that's a workaround for now.

This will take some time, though, and I cannot guarantee a fix...

No worries, take your time to get a good understanding of the issue, and how to apply a fix, I'm not in a hurry.

I love pyfakefs, i've built my test suite on top of it, and it's just awesome 🙂

@mrbean-bremen
Copy link
Member

Thank you 😄

@mrbean-bremen
Copy link
Member

@Wenzel - I decided to make the minimum solution by just patching away the fcntl functions for now, as I'm not sure which part shall be emulated, if any. Your test works with the patch (I forked your repo and pointed it to the branch with the change), if that example passing is sufficient for you I would merge the PR.

@Wenzel
Copy link
Author

Wenzel commented Nov 7, 2021

@mrbean-bremen I confirm that the fix works on my project in Github Actions. 🙂
Thanks you very much for your efforts 💯 ❗

@mrbean-bremen
Copy link
Member

Good to hear! Fixed in master, closing.
Do you need a patch release, ot can you work with master for the time being?

@Wenzel
Copy link
Author

Wenzel commented Nov 7, 2021

@mrbean-bremen i'd rather use an official patch release than rely on the github repo, if that's not too much of a problem for you =)
Many thanks !

@mrbean-bremen
Copy link
Member

Ok, not a problem - the release is out now.

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

No branches or pull requests

2 participants