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

_io and _pyio inconsistenly raise ValueErrors on closed files #98999

Closed
sobolevn opened this issue Nov 2, 2022 · 2 comments
Closed

_io and _pyio inconsistenly raise ValueErrors on closed files #98999

sobolevn opened this issue Nov 2, 2022 · 2 comments
Labels
stdlib Python modules in the Lib dir topic-IO type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Nov 2, 2022

While working on typeshed stubs, I've noticed that _pyio.BufferedReader.read1 does not raise ValueError after .close. While _io.BufferedReader.read1 does that.

So, I went to see both implementation and tests.

C source clearly checks for this here:

CHECK_CLOSED(self, "read of closed file")

But, tests were not executed correctly. Tests were only covering C version: https://github.com/python/cpython/blame/0689b99bb8c4f6058af43a52effaa8a25609dbed/Lib/test/test_io.py#L1532-L1538

    def test_read_on_closed(self):
        # Issue #23796
        b = io.BufferedReader(io.BytesIO(b"12"))
        b.read(1)
        b.close()
        self.assertRaises(ValueError, b.peek)
        self.assertRaises(ValueError, b.read1, 1)

While universal test would look like this:

    def test_read_on_closed(self):
        # Issue #23796
        b = self.BufferedReader(self.BytesIO(b"12"))
        b.read(1)
        b.close()
        self.assertRaises(ValueError, b.peek)
        self.assertRaises(ValueError, b.read1, 1)

And now it is failing:

» ./python.exe -m test -v test_io -m test_read_on_closed
== CPython 3.12.0a1+ (heads/main:7ea10567af, Oct 29 2022, 13:15:59) [Clang 11.0.0 (clang-1100.0.33.16)]
== macOS-10.14.6-x86_64-i386-64bit little-endian
== cwd: /Users/sobolev/Desktop/cpython/build/test_python_78918æ
== CPU count: 4
== encodings: locale=UTF-8, FS=utf-8
0:00:00 load avg: 2.19 Run tests sequentially
0:00:00 load avg: 2.19 [1/1] test_io
test_read_on_closed (test.test_io.CBufferedReaderTest.test_read_on_closed) ... ok
test_read_on_closed (test.test_io.PyBufferedReaderTest.test_read_on_closed) ... FAIL
test_read_on_closed (test.test_io.CBufferedRandomTest.test_read_on_closed) ... ok
test_read_on_closed (test.test_io.PyBufferedRandomTest.test_read_on_closed) ... FAIL

======================================================================
FAIL: test_read_on_closed (test.test_io.PyBufferedReaderTest.test_read_on_closed)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_io.py", line 1537, in test_read_on_closed
    self.assertRaises(ValueError, b.peek)
AssertionError: ValueError not raised by peek

======================================================================
FAIL: test_read_on_closed (test.test_io.PyBufferedRandomTest.test_read_on_closed)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython/Lib/test/test_io.py", line 1537, in test_read_on_closed
    self.assertRaises(ValueError, b.peek)
AssertionError: ValueError not raised by peek

----------------------------------------------------------------------
Ran 4 tests in 0.010s

FAILED (failures=2)

After that, I've tested other methods and here's a list of methods that work on closed in _pyio, but raises on _io:

  • readinto1
  • readinto
  • read1
  • peek

Related https://bugs.python.org/issue23796

Is it a bug or a feature? Is it worth fixing?
This clearly looks like a bug to me.

If others confirm, I have a patch ready :)

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error stdlib Python modules in the Lib dir topic-IO labels Nov 2, 2022
@corona10
Copy link
Member

corona10 commented Nov 2, 2022

@sobolevn It is worth to fix it, Please send the patch.

@sobolevn
Copy link
Member Author

sobolevn commented Nov 2, 2022

Done :)
Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir topic-IO type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants