Python io implementation doesn't flush with write_through=True unlike C implementation

[4kir4]

BPO	21396
Nosy	@pitrou, @4kir4, @vadmium
Files	io-write_through-c-vs-python-issue21396.patch: make C io implementation behave as documented like _pyio already does, add corresponding tests

^{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 2014-05-08.22:34:18.270>
created_at = <Date 2014-04-30.10:36:57.479>
labels = ['type-bug', 'expert-IO']
title = "Python io implementation doesn't flush with write_through=True unlike C implementation"
updated_at = <Date 2014-05-08.22:34:18.268>
user = 'https://github.com/4kir4'

bugs.python.org fields:

activity = <Date 2014-05-08.22:34:18.268>
actor = 'pitrou'
assignee = 'none'
closed = True
closed_date = <Date 2014-05-08.22:34:18.270>
closer = 'pitrou'
components = ['IO']
creation = <Date 2014-04-30.10:36:57.479>
creator = 'akira'
dependencies = []
files = ['35125']
hgrepos = []
issue_num = 21396
keywords = ['patch']
message_count = 7.0
messages = ['217600', '217654', '217684', '218129', '218130', '218132', '218133']
nosy_count = 4.0
nosy_names = ['pitrou', 'akira', 'python-dev', 'martin.panter']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21396'
versions = ['Python 3.4', 'Python 3.5']

[4kir4]

related: msg217596 (bufsize=1 is broken if subprocess module uses Python io)

TextIOWrapper.write behavior:

_pyio.py 1:

        if self._line_buffering and (haslf or "\r" in s):
            self.flush()

textio.c 2:

if (self->write_through)
    needflush = 1;
else if (self->line_buffering &&
    (haslf ||
     PyUnicode_FindChar(text, '\r', 0, PyUnicode_GET_LENGTH(text), 1) != -1))
    needflush = 1;

C implementation calls flush() if write_through=True, Python implementation doesn't.

[pitrou]

Thanks for the report, Akira. Feel free to submit a patch! (with a test)

[4kir4]

I've uploaded the patch that makes C implementation behave according
to the docs like Python implementation with the corresponding tests.

Issue bpo-21332 is a dependency for this issue: subprocess' test_universal_newlines needs to be updated to work with Python version.

For Reference
-------------

issue bpo-12591 introduces write_through to support subprocess' stdin
pipe in text mode with bufsize=0 i.e., TextIOWrapper.buffer is
unbuffered (raw) and Python and C implementation behave the same in
this particular case.

C implementation (pseudo-code)::

  if self.write_through:
      flush_text_buffer()
      buffer.flush() # <-- undocumented

Python implementation::

   if self.write_through:
       pass # _pyio.TextIOWrapper.write() calls buffer.write() directly

behaves according to the current documentation 1:

If *write_through* is ``True``, calls to :meth:`write` are guaranteed
not to be buffered: any data written on the :class:`TextIOWrapper`
object is immediately handled to its underlying binary *buffer*

For reference, here's how subprocess.py uses write_through 2::

  self.stdin = io.open(pipe, 'wb', bufsize)
  if universal_newlines=True:
      self.stdin = io.TextIOWrapper(self.stdin,
          write_through=True,
          line_buffering=(bufsize == 1)) # <-- issue python/cpython#65531

http://hg.python.org/cpython/rev/9ce8fa0a0899/ - introduce io.TextIOWrapper in subprocess
http://hg.python.org/cpython/rev/5cc536fbd7c1 - introduce write_through

[pitrou]

The patch looks basically fine. I will make a few tweaks to the comments in the test case.

[pitrou]

Actually, with the patch, the universal_newlines tests in test_subprocess hang (quite logically, since they lack a flush()). I will fix them as well.

[python-dev]

New changeset 39f2a78f4357 by Antoine Pitrou in branch '3.4':
Issue bpo-21396: Fix TextIOWrapper(..., write_through=True) to not force a flush() on the underlying binary stream.
http://hg.python.org/cpython/rev/39f2a78f4357

New changeset 37d0c41ed8ad by Antoine Pitrou in branch 'default':
Issue bpo-21396: Fix TextIOWrapper(..., write_through=True) to not force a flush() on the underlying binary stream.
http://hg.python.org/cpython/rev/37d0c41ed8ad

[pitrou]

Thank you for the patch!