pytest-dev / pyfakefs

Provides a fake file system that mocks the Python file system modules.
https://pytest-pyfakefs.readthedocs.io
Apache License 2.0
627 stars 88 forks source link

OSError opening same fd twice #997

Closed carlmontanari closed 2 months ago

carlmontanari commented 2 months ago

Describe the bug Describe the observed behavior, and what you expect to happen instead. In case of a crash, please provide the complete stack trace.

:wave: in the recent release there were some changes around opening fake files and checking their permissions in #967 which resulted in #983 it looks like.

I have a situation (repro below) where I open the same fd twice and create an io.BufferedRWPair -- with this latest update this fails with the below exception as the file is already opened once.

To be honest im not sure this is a bug or if what I have is not very sane/smart -- the relevant bits for me where vendor'd from pexepct/ptyprocess and ive pretty much left it alone for forever :)

What is/was the reason for only allowing things that aren't pypy to open a file once?

        newline, open_modes = self._handle_file_mode(mode, newline, open_modes)
        opened_as_fd = isinstance(file_, int)
        if opened_as_fd and not helpers.IS_PYPY:
            fd: int = cast(int, file_)
            # cannot open the same file descriptor twice, except in PyPy
            for f in self.filesystem.get_open_files(fd):
                if isinstance(f, FakeFileWrapper) and f.opened_as_fd:
>                   raise OSError(errno.EBADF, "Bad file descriptor")
E                   OSError: [Errno 9] Bad file descriptor

venv/lib/python3.11/site-packages/pyfakefs/fake_open.py:152: OSError

How To Reproduce Please provide a unit test or a minimal reproducible example that shows the problem.

from io import BufferedRWPair
from pyfakefs.fake_filesystem_unittest import Patcher

def test_repro():
    with Patcher(use_cache=False) as patcher:
        fs_ = patcher.fs

        fs_.create_file("dummy")
        dummy_file = open("/dummy")

        fd = dummy_file.fileno()

        readf = open(fd, "rb", buffering=0)
        writef = open(fd, "wb", buffering=0, closefd=False)
        _ = BufferedRWPair(readf, writef)

Your environment Please run the following in the environment where the problem happened and paste the output.

❯ python -c "import platform; print(platform.platform())"
macOS-14.4.1-arm64-arm-64bit
❯ python -c "import sys; print('Python', sys.version)"
Python 3.11.9 (main, Apr  2 2024, 08:25:04) [Clang 15.0.0 (clang-1500.3.9.4)]
❯ python -c "from pyfakefs import __version__; print('pyfakefs', __version__)"
pyfakefs 5.4.0
❯ python -c "import pytest; print('pytest', pytest.__version__)"
pytest 7.4.4

Thanks a bunch for this project, been using it in testing for a long time and its always been so nice and easy to work with!

carl

mrbean-bremen commented 2 months ago

Thanks! I somewhat expected new issues to crop up with that change, as the tests cover only a small part of the possible usages...

What is/was the reason for only allowing things that aren't pypy to open a file once?

That was just an observation of the behavior in the real file system, at least with the tests I ran. Pypi often behave slightly different compared to CPython - I did not investigate the cause.

You probably have to pin pyfakefs to the previous version as long as this is not fixed. I'm not sure about the timeline for the fix, these issues tend to be a bit tricky - I will let you know as soon as I understand the problem.

mrbean-bremen commented 2 months ago

Hm, I'm having trouble understanding the behavior in the real file system. If I create a file my_file, and do:

    fd = my_file.fileno()
    readf = open(fd, "rb", buffering=0)
    writef = open(fd, "wb", buffering=0, closefd=False)
    os.close(fd)

this works. If I do instead:

    fd = my_file.fileno()
    open(fd, "rb", buffering=0)
    open(fd, "wb", buffering=0, closefd=False)
    os.close(fd)

e.g. the same, but not assigning the created file to a variable, I get an OSError (invalid file handle) on the second open call. This is the behavior, that pyfakefs simulates and tests.

I'm just recording this for reference, I'm a bit stumped at the moment...

mrbean-bremen commented 2 months ago

After writing this, I realized that this means that the file is closed if it is not assigned as in the second case, which explains the invalid file descriptor. This explains the behavior, though I will have to rethink the logic here - I'm not sure how to correctly implement this. This may take some time...

mrbean-bremen commented 2 months ago

So, to sum this up: I will remove the ugly code that caused this and reopen the issue which I had fixed by that.

Now that I understand that this is a reference counting / garbage collection problem, it is also clear why the behavior is different in pypy. The file is closed if the file object is released, which only can happen immediately after the line it was created, if the file is a C object. With a Python implementation (pypy) this will happen later when the garbage collector kicks in.

I have no idea yet what to do about that other issue without adding C code to pyfakefs (which I really don't want to), but that is something unrelated to this issue.

mrbean-bremen commented 2 months ago

Actually I don't have to reopen the other issue, as this specific behavior was not a part of the issue. Looks I found this myself while testing, and misunderstood the behavior. The case that it was supposed to fix is probably not relevant, as the code makes no real sense, so I will just ignore it. It was all just a tempest in a teacup... (well, mostly).

mrbean-bremen commented 2 months ago

@carlmontanari - I'm going to handle another issue first, and make a new release after that.

mrbean-bremen commented 2 months ago

The other issue may take a bit longer, so I decided to make a patch release now - done.