Skip to content

Commit

Permalink
Remove unneeded check leading to a regression
Browse files Browse the repository at this point in the history
- the check had been incorrecty introduced to fix an issue
  ultimately related to reference counting/garbage collection
- the fix was incorrect, and that case can probably be ignored,
  as the code leading to it makes no sense
  • Loading branch information
mrbean-bremen committed Apr 10, 2024
1 parent 7041a39 commit af2bbb2
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 30 deletions.
6 changes: 6 additions & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
# pyfakefs Release Notes
The released versions correspond to PyPI releases.

## Unreleased

### Fixes
* fixed a regression from version 5.4.0 that incorrectly handled files opened twice via file descriptor
(see [#997](../../issues/997))

## [Version 5.4.0](https://pypi.python.org/pypi/pyfakefs/5.4.0) (2024-04-07)
Improves permission handling.

Expand Down
6 changes: 0 additions & 6 deletions pyfakefs/fake_open.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,6 @@ def call(

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")

# the pathlib opener is defined in a Path instance that may not be
# patched under some circumstances; as it just calls standard open(),
Expand Down
32 changes: 8 additions & 24 deletions pyfakefs/tests/fake_os_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,18 +231,18 @@ def test_closed_file_descriptor(self):
self.assert_raises_os_error(errno.EBADF, self.os.fdopen, fileno2, **kwargs)

def test_fdopen_twice(self):
# regression test for #997
file_path = self.make_path("some_file1")
self.create_file(file_path, contents="contents here1")
fake_file = self.open(file_path, "r", encoding="utf8")
fd = fake_file.fileno()
self.open(fd, encoding="utf8")
if not IS_PYPY:
with self.assertRaises(OSError) as cm:
self.open(fd, encoding="utf8")
self.assertEqual(errno.EBADF, cm.exception.errno)
else:
self.open(fd, encoding="utf8")
self.os.close(fd)
# note: we need to assign the files to objects,
# otherwise the file will be closed immediately in the CPython implementation
# note that this case is not (and will probably not be) handled in pyfakefs
file1 = self.open(fd, encoding="utf8") # noqa: F841
file2 = self.open(fd, encoding="utf8") # noqa: F841

self.os.close(fd)

def test_open_fd_write_mode_for_ro_file(self):
# Create a writable file handle to a read-only file, see #967
Expand Down Expand Up @@ -3159,22 +3159,6 @@ def test_listdir_impossible_without_read_permission(self):
with self.open(file_path, encoding="utf8") as f:
assert f.read() == "hey"

def test_fdopen_twice(self):
file_path1 = self.make_path("some_file1")
file_path2 = self.make_path("SOME_file1")
self.create_file(file_path1, contents="contents here1")

fake_file1 = self.open(file_path2, "r", encoding="utf8")
fileno1 = fake_file1.fileno()
self.os.fdopen(fileno1, encoding="utf8")
if not IS_PYPY:
with self.assertRaises(OSError) as cm:
self.open(fileno1, encoding="utf8")
self.assertEqual(errno.EBADF, cm.exception.errno)
else:
self.open(fileno1, encoding="utf8")
self.os.close(fileno1)

def test_stat(self):
directory = self.make_path("xyzzy")
directory1 = self.make_path("XYZZY")
Expand Down

0 comments on commit af2bbb2

Please sign in to comment.