PEP 475: Add _Py_read() and _Py_write() functions

[vstinner]

BPO	23708
Nosy	@vstinner
Files	py_read_write.patch select_write.patch py_read_write-2.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-03-20.11:24:27.285>
created_at = <Date 2015-03-19.13:33:30.591>
labels = []
title = 'PEP 475: Add _Py_read() and _Py_write() functions'
updated_at = <Date 2015-03-20.11:24:27.251>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2015-03-20.11:24:27.251>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2015-03-20.11:24:27.285>
closer = 'vstinner'
components = []
creation = <Date 2015-03-19.13:33:30.591>
creator = 'vstinner'
dependencies = []
files = ['38557', '38558', '38562']
hgrepos = []
issue_num = 23708
keywords = ['patch']
message_count = 14.0
messages = ['238514', '238515', '238517', '238526', '238528', '238569', '238571', '238572', '238576', '238638', '238639', '238640', '238642', '238647']
nosy_count = 2.0
nosy_names = ['vstinner', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue23708'
versions = ['Python 3.5']

[vstinner]

To factorize code handling EINTR, I propose add 2 new functions to fileutils.h: _Py_read() and _Py_write().

Attached patch adds these functions and use them in the os and _io modules.

The code of the functions is based on the code from the os and _io modules. The main change is that the functions now truncate code if greater than PY_SSIZE_T_MAX. Other changes are just refactoring.

[vstinner]

select_write.patch: use _Py_write() in the select module.

[vstinner]

I created the issue bpo-23709 for the ossaudiodev module: "Refactor ossaudiodev: use _Py_read and _Py_write with the Py_buffer".

[vstinner]

With py_read_write.patch in debug mode, test_threading crash with an assertion error because of the issue bpo-15751. Currently, it's not possible to call assert(PyGILState_Check()); inside Py_NewInterpreter(), while creating a subinterpreter (ex: _testcapi.run_in_subinterp()).

The workaround is to comment the assertion in _Py_read() and Py_write() until the issue bpo-15751 is fixed.

[vstinner]

Oops, there was a bug in FileIO.readall(), fixed in the new patch py_read_write-2.patch.

I also commented assert(PyGILState_Check()); to workaround the issue bpo-15751.

[python-dev]

New changeset c3c47ea32f72 by Victor Stinner in branch 'default':
Issue bpo-23708: Add _Py_read() and _Py_write() functions to factorize code handle
https://hg.python.org/cpython/rev/c3c47ea32f72

[python-dev]

New changeset e232b57ee784 by Victor Stinner in branch 'default':
Issue bpo-23708: select.devpoll now retries its internal write() when interrupted
https://hg.python.org/cpython/rev/e232b57ee784

[vstinner]

I commited both patches. Thanks Antoine for the review of py_read_write-2.patch, I removed the commented assertion.

[python-dev]

New changeset 116e4c40115f by Victor Stinner in branch 'default':
Issue bpo-23708: Fix _Py_read() compilation error on Windows
https://hg.python.org/cpython/rev/116e4c40115f

[vstinner]

An assertion failed in _Py_read() while running test_signal on AMD64 Snow Leop 3.x:

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

[321/393/1] test_signal
Assertion failed: (errno == EINTR && PyErr_Occurred()), function _Py_read, file Python/fileutils.c, line 1181.
Fatal Python error: Aborted

Current thread 0x00007fff71296cc0 (most recent call first):
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/subprocess.py", line 1407 in _execute_child
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/subprocess.py", line 855 in __init__
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_signal.py", line 126 in run_test
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_signal.py", line 180 in test_main
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 1772 in _run_suite
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/support/init.py", line 1806 in run_unittest
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_signal.py", line 1123 in test_main
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/regrtest.py", line 1284 in runtest_inner
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/regrtest.py", line 967 in runtest
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/regrtest.py", line 532 in main
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/regrtest.py", line 1568 in main_in_temp_cwd
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/regrtest.py", line 1593 in <module>
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/runpy.py", line 85 in _run_code
File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/runpy.py", line 170 in _run_module_as_main

[python-dev]

New changeset 6dd201b6bb4f by Victor Stinner in branch 'default':
Issue bpo-23708: Split assertion expression in two assertions in _Py_read() and
https://hg.python.org/cpython/rev/6dd201b6bb4f

[vstinner]

Same error on "x86 Tiger 3.x" buildbot:

http://buildbot.python.org/all/builders/x86%20Tiger%203.x/builds/9381/steps/test/logs/stdio

[345/393] test_signal
Python/fileutils.c:1181: failed assertion `errno == EINTR && PyErr_Occurred()'
Fatal Python error: Aborted

Current thread 0xa000d000 (most recent call first):
File "/Users/db3l/buildarea/3.x.bolen-tiger/build/Lib/subprocess.py", line 1407 in _execute_child
File "/Users/db3l/buildarea/3.x.bolen-tiger/build/Lib/subprocess.py", line 855 in __init__
File "/Users/db3l/buildarea/3.x.bolen-tiger/build/Lib/test/test_signal.py", line 126 in run_test
...

[python-dev]

New changeset 07fd54208434 by Victor Stinner in branch 'default':
Issue bpo-23708: Save/restore errno in _Py_read() and _Py_write()
https://hg.python.org/cpython/rev/07fd54208434

[vstinner]

New changeset 07fd54208434 by Victor Stinner in branch 'default':
Issue bpo-23708: Save/restore errno in _Py_read() and _Py_write()

When I wrote _Py_read()/_Py_write(), I added assertions on errno to ensure that errno is not modified. I chose to use assertions instead of save/restore errno because on Linux errno is not modified. Since they *are* platforms where errno is modified, it's safer to always save/restore errno.

_Py_read()/_Py_write() *must* set set errno because some callers uses it. It's more convinient to use errno than having to get the errno attribute of the raised OSError exception.

test_signal pass again on "AMD64 Snow Leop 3.x" buildbot, I close the issue.