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

python3.0 -u: unbuffered stdout #48955

Closed
vstinner opened this issue Dec 20, 2008 · 26 comments
Closed

python3.0 -u: unbuffered stdout #48955

vstinner opened this issue Dec 20, 2008 · 26 comments
Assignees
Labels
release-blocker type-bug An unexpected behavior, bug, or error

Comments

@vstinner
Copy link
Member

BPO 4705
Nosy @birkenfeld, @mdickinson, @pitrou, @vstinner, @fabioz, @benjaminp, @joedborg
Files
  • unbuffered-stdin.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 = 'https://github.com/pitrou'
    closed_at = <Date 2009-01-26.22:09:35.831>
    created_at = <Date 2008-12-20.13:47:22.003>
    labels = ['type-bug', 'release-blocker']
    title = 'python3.0 -u: unbuffered stdout'
    updated_at = <Date 2013-08-26.16:45:16.771>
    user = 'https://github.com/vstinner'

    bugs.python.org fields:

    activity = <Date 2013-08-26.16:45:16.771>
    actor = 'Joe.Borg'
    assignee = 'pitrou'
    closed = True
    closed_date = <Date 2009-01-26.22:09:35.831>
    closer = 'pitrou'
    components = []
    creation = <Date 2008-12-20.13:47:22.003>
    creator = 'vstinner'
    dependencies = []
    files = ['12798']
    hgrepos = []
    issue_num = 4705
    keywords = ['patch']
    message_count = 26.0
    messages = ['78102', '78127', '78387', '78393', '78395', '78400', '78403', '78408', '78417', '78420', '78886', '78922', '79444', '79449', '79490', '80190', '80195', '80539', '80540', '80541', '80543', '80544', '80575', '80590', '80597', '196224']
    nosy_count = 7.0
    nosy_names = ['georg.brandl', 'mark.dickinson', 'pitrou', 'vstinner', 'fabioz', 'benjamin.peterson', 'Joe.Borg']
    pr_nums = []
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue4705'
    versions = ['Python 3.0', 'Python 3.1', 'Python 3.2', 'Python 3.3']

    @vstinner
    Copy link
    Member Author

    I like and I need an "unbuffered" standard output which was provided
    by -u command line option (or PYTHONUNBUFFERED environment variable).
    Current status of -u option in Python3: the option exists and change
    the buffer size (disable buffering) of the stdin, stdout and stderr
    file descriptors.

    The problem is in initstdio() which creates files with buffering=-1
    (default buffer) instead of buffering=0 (no buffering) or buffering=1
    (line buffer). But open() enable line buffering of TextIOWrapper is
    buffering=-1 and the raw file is a tty.

    Example with py3k trunk:
    ------------

    $ ./python
    >>> import sys; sys.stdout.line_buffering
    True
    $ ./python |cat
    >>> import sys; sys.stdout.line_buffering
    False

    I would like line buffering when stdout is redirected to a pipe and -u
    option is used. initstdio() have to be changed to choose buffering
    option. So it's something like:

    Index: Python/pythonrun.c
    ===================================================================

    --- Python/pythonrun.c  (révision 67870)
    +++ Python/pythonrun.c  (copie de travail)
    @@ -810,7 +810,12 @@
     #endif
            }
            else {
    -               if (!(std = PyFile_FromFd(fd, "<stdout>", "w", -1, 
    encoding,
    +               int buffering;
    +               if (1)
    +                       buffering = 1; /* line */
    +               else
    +                       buffering = -1; /* default */
    +               if (!(std = PyFile_FromFd(fd, "<stdout>", "w", 
    buffering, encoding,
                                              errors, "\n", 0))) {
                            goto error;
                    }

    But "if (1)" have to be replaced "if -u option is used" :-) See
    unbuffered variable of Modules/main.c.

    @fabioz
    Copy link
    Mannequin

    fabioz mannequin commented Dec 20, 2008

    Just as a note, Pydev needs the unbuffered output (or it cannot get it).
    This has been brought up in the python-dev list:
    http://mail.python.org/pipermail/python-dev/2008-December/084436.html

    As a workaround for now I'm using:
    sys.stdout._line_buffering = True,

    but that doesn't seem right as it's accessing an internal attribute.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 27, 2008

    Here is a patch.

    @pitrou pitrou added the type-bug An unexpected behavior, bug, or error label Dec 27, 2008
    @vstinner
    Copy link
    Member Author

    pitrou's patch changes PyFile_FromFd() behaviour for a text file
    opened with buffering=0:
    /* As a convenience, when buffering == 0 on a text file, we
    open the underlying binary stream in unbuffered mode and
    wrap it with a text stream in line-buffered mode. */

    Why changing PyFile_FromFd() and not io.open() directly?

    Note: I prefer Py_UnbufferedStdoutFlag=1 instead of
    Py_UnbufferedStdoutFlag++ (for -u command line option).

    Except the minor comments, I like the patch (and it has unit
    tests!) ;-)

    @pitrou
    Copy link
    Member

    pitrou commented Dec 28, 2008

    pitrou's patch changes PyFile_FromFd() behaviour for a text file
    opened with buffering=0:
    /* As a convenience, when buffering == 0 on a text file, we
    open the underlying binary stream in unbuffered mode and
    wrap it with a text stream in line-buffered mode. */

    Why changing PyFile_FromFd() and not io.open() directly?

    I must admit I'm a bit lazy, and changing io.open() means changing a
    fundamental public API, as Guido said on python-dev, so more discussion
    and some parts of the patches delayed to 3.1. If someone else wants to
    do it, please don't hesitate...

    Note: I prefer Py_UnbufferedStdoutFlag=1 instead of
    Py_UnbufferedStdoutFlag++ (for -u command line option).

    Well, I minimally changed the existing code.

    @vstinner
    Copy link
    Member Author

    > Why changing PyFile_FromFd() and not io.open() directly?

    I must admit I'm a bit lazy, and changing io.open() means changing
    a fundamental public API, as Guido said on python-dev, so
    more discussion and some parts of the patches delayed to 3.1.

    You're right, and PyFile_FromFd() is also a fundamental "public" API.
    Since TextIOWrapper doesn't support real unbuffered buffer (only
    pseudo line buffer: unbuffered raw buffer and line buffering for
    TextIOWrapper), I prefer to change only stdout/stderr instead of
    PyFile_FromFd().

    My new patch only changes initstdio() using pitrou's code.

    Should we also change stdin?

    @pitrou
    Copy link
    Member

    pitrou commented Dec 28, 2008

    Le dimanche 28 décembre 2008 à 12:19 +0000, STINNER Victor a écrit :

    STINNER Victor <victor.stinner@haypocalc.com> added the comment:

    > > Why changing PyFile_FromFd() and not io.open() directly?
    >
    > I must admit I'm a bit lazy, and changing io.open() means changing
    > a fundamental public API, as Guido said on python-dev, so
    > more discussion and some parts of the patches delayed to 3.1.

    You're right, and PyFile_FromFd() is also a fundamental "public" API.

    Well, open() is fundamental as in part of the built-ins and used
    pervasively. PyFile_FromFd(), on the other hand, is a relic of the 2.x C
    file handling API. Let's see what others have to say about this.

    Should we also change stdin?

    I don't know, but "python -h" only talks about stderr/stdout.

    @pitrou
    Copy link
    Member

    pitrou commented Dec 28, 2008

    It seems the "name" field of the TextIOWrapper object isn't set in
    create_stdio() (the "char *name" parameter isn't used). Otherwise, the
    patch looks good.

    @vstinner
    Copy link
    Member Author

    > Should we also change stdin?
    I don't know, but "python -h" only talks about stderr/stdout.

    The manpage of Python2 is clear:

    -u Force stdin, stdout and stderr to be totally unbuffered.

    stdin is also unbuffered.

    It seems the "name" field of the TextIOWrapper object isn't
    set in create_stdio()

    It used only used for buffered output. Without the patch,
    sys.stdout.name == sys.stdout.buffer.name == '1' :-/

    New patch:

    • use create_stdio() to create unbuffered sys.stdin
    • rename Py_UnbufferedStdoutFlag to Py_UnbufferedStdioFlag
    • replace "Py_UnbufferedStdioFlag++;" by "Py_UnbufferedStdioFlag =
      1;"
    • change create_stdio(): (...)

    Note: there is no test for unbuffered input because I don't know how
    to test this (even by manual tests) :-p

    @vstinner
    Copy link
    Member Author

    Attached: quick and dirty test to check if the standard input is
    buffered or not. My short test program works with python2.5 and py3k
    trunk without the -u command line option. So changing sys.stdin buffer
    is not really important.

    About the wrong name, I opened a separated issue: bpo-4762,
    PyFile_FromFd() doesn't set the file name.

    @benjaminp
    Copy link
    Contributor

    Instead of importing IO each time in create_stdio, maybe you should just
    pass io.open to create_stdio.

    @vstinner
    Copy link
    Member Author

    vstinner commented Jan 3, 2009

    Instead of importing IO each time in create_stdio,
    maybe you should just pass io.open to create_stdio

    create_stdio() uses io.open() but also io.TextIOWrapper. Since io
    module is already imported in initstdio(), I updated the patch to just
    pass the pointer to the module to create_stdio().

    @pitrou
    Copy link
    Member

    pitrou commented Jan 8, 2009

    If PyObject_SetAttrString(raw, "_name", text) fails, a reference to
    raw is leaked.
    Other than that, the patch looks good.

    @vstinner
    Copy link
    Member Author

    vstinner commented Jan 8, 2009

    Updated patch: clear raw on error
    + if (!Py_UnbufferedStdioFlag)
    + Py_XDECREF(raw);

    Question: Should we use line_buffering in unbuffered mode?

    @pitrou
    Copy link
    Member

    pitrou commented Jan 9, 2009

    Committed in r68451. Thanks!

    @pitrou pitrou closed this as completed Jan 9, 2009
    @pitrou
    Copy link
    Member

    pitrou commented Jan 19, 2009

    Reopening, since sys.stdin is actually broken in unbuffered mode:

    $ ./python -u
    Python 3.1a0 (py3k:68756, Jan 19 2009, 01:17:26) 
    [GCC 4.3.2] on linux2
    Type "help", "copyright", "credits" or "license" for more information.
    >>> import sys
    >>> sys.stdin.read(1)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/antoine/py3k/__svn__/Lib/io.py", line 1739, in read
        eof = not self._read_chunk()
      File "/home/antoine/py3k/__svn__/Lib/io.py", line 1565, in _read_chunk
        input_chunk = self.buffer.read1(self._CHUNK_SIZE)
    AttributeError: 'FileIO' object has no attribute 'read1'
    >>> 

    What I propose is that stdin be always opened in buffered mode (even
    with -u), since I don't see how the behaviour can differ for a read-only
    non-seekable stream.

    @pitrou pitrou reopened this Jan 19, 2009
    @pitrou
    Copy link
    Member

    pitrou commented Jan 19, 2009

    Here is a patch.

    @mdickinson
    Copy link
    Member

    since I don't see how the behaviour can differ for a read-only
    non-seekable stream.

    Unless I'm misunderstanding you (quite likely), I think one *can* get
    different results with buffered and unbuffered stdin.
    For example, on my machine, if I create the following script:

    #!/usr/bin/python -u
    import sys
    print sys.stdin.readline()

    and name it test.py, I get the following result in an OS X Terminal
    running bash:

    dickinsm$ ls python_source/trunk/Objects/ | (./test.py; ./test.py)
    abstract.c

    boolobject.c

    Whereas if I remove the '-u' from the shebang line I just get:

    dickinsm$ ls python_source/trunk/Objects/ | (./test.py; ./test.py)
    abstract.c

    I'm not 100% sure that I understand exactly what's going on here, but it's
    something like the following: in the first (unbuffered) case, the
    stdin.readline call of the first ./test.py only reads one line from stdin,
    leaving the rest intact; so the second ./test.py also gets to output a
    line. In the second case some larger amount of stdin (1024 bytes?) is
    immediately slurped into the stdin buffer for the first Python process, so
    the second ./test.py doesn't get anything.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 25, 2009

    [...]

    I hadn't thought of such situations :-/

    So the question is whether it is really useful to enforce unbuffered
    stdin with the '-u' option (or your example is simply too borderline).
    If so, the patch will have to be replaced with another one implementing
    read1() in the FileIO class.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 25, 2009

    Thinking about it, TextIOWrapper has its own input buffering (the
    decoded_chars attribute), so your use case would probably not be
    satisfied.

    (and disabling TextIOWrapper's internal buffering would be a bad idea
    since it would make it horribly slow)

    @mdickinson
    Copy link
    Member

    So the question is whether it is really useful to enforce unbuffered
    stdin with the '-u' option (or your example is simply too borderline).

    Hard to say. It seems at least possible that there are Python users for
    whom stdin being unbuffered (with -u) matters, so if there's any
    reasonable way of avoiding changing this it should probably be considered.

    Though I have to admit that I'm not one of those users (I don't think I've
    *ever* used the -u option outside of testing...).

    @pitrou
    Copy link
    Member

    pitrou commented Jan 25, 2009

    Hard to say. It seems at least possible that there are Python users for
    whom stdin being unbuffered (with -u) matters, so if there's any
    reasonable way of avoiding changing this it should probably be considered.

    It's not about changing it, stdin has always been buffered in py3k. My
    original commit actually attempted to change it, and it failed (I hadn't
    noticed it at first due to mis-testing on my part). The new patch is
    about putting it back in buffered mode even with '-u'.

    @mdickinson
    Copy link
    Member

    It's not about changing it, stdin has always been buffered in py3k.

    Sorry: I should have been clearer. It's the change from 2.x to 3.x that
    interests me.

    So 'python3.0 -u' has buffered stdin, while 'python2.6 -u' does not; I'm
    wondering: was this an intentional design change? Or was it just an
    accident/by-product of the rewritten io?

    Anyway, the patch looks good to me.

    @pitrou
    Copy link
    Member

    pitrou commented Jan 26, 2009

    So 'python3.0 -u' has buffered stdin, while 'python2.6 -u' does not;
    I'm wondering: was this an intentional design change? Or was it just
    an accident/by-product of the rewritten io?

    I'm not sure (I didn't write the new io in the first place) but I'd say
    it was simply overlooked. Otherwise 'python3.0 -u' would have had at
    least unbuffered stdout/stderr, which it didn't have.

    @pitrou pitrou self-assigned this Jan 26, 2009
    @pitrou
    Copy link
    Member

    pitrou commented Jan 26, 2009

    Committed and applied a small fix to the test so that it passes in debug
    mode (r68977, r68981, r68982). Thanks!

    @pitrou pitrou closed this as completed Jan 26, 2009
    @joedborg
    Copy link
    Mannequin

    joedborg mannequin commented Aug 26, 2013

    Can I confirm this is still in the trunk? I have 3.3.2 and am suffering from the fact that -u isn't setting stdin to unbuffered. I'm have to run a flush every command, which is awful.

    @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
    Labels
    release-blocker type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants