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.

Sign up for GitHub

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

Jump to bottom

Cleanup io.FileIO #67940

Closed
vstinner opened this issue Mar 23, 2015 · 10 comments
Closed

Cleanup io.FileIO #67940

vstinner opened this issue Mar 23, 2015 · 10 comments
Labels
stdlib Python modules in the Lib dir

Comments

@vstinner
Copy link
Member

BPO 23752
Nosy @vstinner
Files
  • fileio_drop_dup_fstat.patch
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = <Date 2015-04-06.23:38:35.778>
created_at = <Date 2015-03-23.16:51:05.752>
labels = ['library']
title = 'Cleanup io.FileIO'
updated_at = <Date 2015-04-06.23:38:35.777>
user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2015-04-06.23:38:35.777>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2015-04-06.23:38:35.778>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2015-03-23.16:51:05.752>
creator = 'vstinner'
dependencies = []
files = ['38661']
hgrepos = []
issue_num = 23752
keywords = ['patch']
message_count = 10.0
messages = ['239045', '239063', '239066', '239070', '239548', '239577', '239578', '239588', '239589', '240193']
nosy_count = 4.0
nosy_names = ['holdenweb', 'vstinner', 'python-dev', 'piotr.dobrogost']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue23752'
versions = ['Python 3.5']
    @vstinner
    Copy link
    Member Author

    While reviewing a Python implementation of io.FileIO (_pyio.FileIO) in the issue bpo-21859, many issues were found in the C implementation. I open an issue to fix them.

    We may fix them before or after the Python implementation is merged.

    @vstinner vstinner added the stdlib Python modules in the Lib dir label Mar 23, 2015
    @vstinner
    Copy link
    Member Author

    fileio_drop_dup_fstat.patch: Remove the duplicate call to fstat() in fileio.c. Tests pass, invalid FD are still detected since there is a second call to fstat() which also raises OSError(EBADF) if fstat(fd) fails.

    @vstinner
    Copy link
    Member Author

    The previous change related to fstat() is the changeset 3b5279b5bfd1 from the issue bpo-21679. The changeset introduces the private _blksize attribute. The strange thing is that the changelog is:
    "Issue bpo-21679: Prevent extraneous fstat() calls during open(). Patch by Bohuslav Kabrda."

    fstat() was called twice and it is still called twice. Maybe I missed something.

    @vstinner
    Copy link
    Member Author

    See also issue bpo-21861: "io class name are hardcoded in reprs".

    In my review of _pyio.FileIO, I asked if it would be possible to use __qualname__ in __getstate__().

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 30, 2015

    New changeset bc2a22eaa0af by Victor Stinner in branch 'default':
    Issue bpo-23752: When built from an existing file descriptor, io.FileIO() now only
    https://hg.python.org/cpython/rev/bc2a22eaa0af

    @vstinner
    Copy link
    Member Author

    Hum, _Py_fstat() is not used correctly in _io.FilIO: it uses errno instead of GetLastError() to raise the OSError. It's too hard to use the new _Py_fstat() function directly, I modified it to raise directly the exception. So the caller doesn't have to use #ifdef MS_WINDOWS to raise the exception correctly.

    I added _Py_fstat_noraise() when it's not possible to use an exception.

    http://buildbot.python.org/all/builders/AMD64%20Windows7%20SP1%203.x/builds/5978/steps/test/logs/stdio

    ======================================================================
    FAIL: test_fdopen (test.test_os.TestInvalidFD)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_os.py", line 1418, in check
    f(support.make_bad_fd(), *args)
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\os.py", line 1028, in fdopen
    return io.open(fd, *args, **kwargs)
OSError: [Errno 0] Error

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_os.py", line 1411, in helper
    self.check(getattr(os, f))
  File "C:\buildbot.python.org\3.x.kloth-win64\build\lib\test\test_os.py", line 1420, in check
    self.assertEqual(e.errno, errno.EBADF)
AssertionError: 0 != 9

    @vstinner vstinner reopened this Mar 30, 2015
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Mar 30, 2015

    New changeset 128f92ae8bae by Victor Stinner in branch 'default':
    Issue bpo-23752: _Py_fstat() is now responsible to raise the Python exception
    https://hg.python.org/cpython/rev/128f92ae8bae

    @vstinner
    Copy link
    Member Author

    The changeset 128f92ae8bae also changed _Py_fstat() to release the GIL when calling fstat(), I forgot to mention it in the changeset.

    test_socket now hangs on AMD64 Snow Leop 3.x, I don't know yet if it's related:

    http://buildbot.python.org/all/builders/AMD64%20Snow%20Leop%203.x/builds/2870/steps/test/logs/stdio

    [393/393] test_socket
    Timeout (1:00:00)!
    Thread 0x0000000104fef000 (most recent call first):
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/socket.py", line 288 in _sendfile_use_sendfile
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_socket.py", line 5131 in _testOffset
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_socket.py", line 278 in clientRun

    Thread 0x00007fff71296cc0 (most recent call first):
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/threading.py", line 293 in wait
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/threading.py", line 556 in wait
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_socket.py", line 262 in _tearDown
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/case.py", line 580 in run
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/case.py", line 625 in __call__
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/suite.py", line 122 in run
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/suite.py", line 84 in __call__
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/suite.py", line 122 in run
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/suite.py", line 84 in __call__
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/runner.py", line 176 in run
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/support/init.py", line 1773 in _run_suite
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/support/init.py", line 1807 in run_unittest
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_socket.py", line 5340 in test_main
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/regrtest.py", line 1280 in runtest_inner

    @vstinner
    Copy link
    Member Author

    Oh, there was another hang before, so it's probably not related:

    http://buildbot.python.org/all/builders/AMD64%20Snow%20Leop%203.x/builds/2852/steps/test/logs/stdio

    [393/393] test_ssl
    Timeout (1:00:00)!
    Thread 0x0000000102d81000 (most recent call first):
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_ssl.py", line 2832 in serve
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/threading.py", line 871 in run
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/threading.py", line 923 in _bootstrap_inner
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/threading.py", line 891 in _bootstrap

    Thread 0x00007fff71296cc0 (most recent call first):
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/ssl.py", line 628 in do_handshake
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/ssl.py", line 983 in do_handshake
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/ssl.py", line 747 in __init__
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/ssl.py", line 1064 in wrap_socket
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/case.py", line 162 in handle
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/case.py", line 1238 in assertRaisesRegex
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_ssl.py", line 2851 in test_handshake_timeout
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/case.py", line 577 in run
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/case.py", line 625 in __call__
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/suite.py", line 122 in run
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/suite.py", line 84 in __call__
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/suite.py", line 122 in run
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/suite.py", line 84 in __call__
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/unittest/runner.py", line 176 in run
    File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/support/init.py", line 1773 in _run_suite

    @vstinner
    Copy link
    Member Author

    vstinner commented Apr 6, 2015

    Buildbot issues were unrelated and have been fixed. I close the issue.

    @vstinner vstinner closed this as completed Apr 6, 2015
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees
    No one assigned
    Labels
    stdlib Python modules in the Lib dir
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @vstinner