classification
Title: input() uses sys.__stdout__ instead of sys.stdout for prompt
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.6, Python 3.4, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: martin.panter Nosy List: Keita Kita, berker.peksag, eryksun, martin.panter, pitrou, python-dev, serhiy.storchaka, taleinat, xiang.zhang
Priority: normal Keywords: patch

Created on 2015-06-07 09:30 by Keita Kita, last changed 2015-10-10 10:04 by martin.panter. This issue is now closed.

Files
File name Uploaded Description Edit
issue24402.fix_only.patch taleinat, 2015-06-12 21:51 patch with just a fix (no tests) review
input-terminal.patch martin.panter, 2015-10-07 09:28 Now with test case review
Messages (15)
msg244949 - (view) Author: Keita Kita (Keita Kita) Date: 2015-06-07 09:30
On Python 3.4, input() does not use wrapped sys.stdout. For example, the following script should print "Wrapped stdout" by print() and input() because sys.stdout is replaced with Writer object.

--------
import io
import sys


class Writer:
    def __getattr__(self, name):
        return getattr(sys.__stdout__, name)

    def write(self, data):
        if data != '\n':
            sys.__stdout__.write('Wrapped stdout\n')

    def fileno():
        raise OSError()


sys.stdout = Writer()

print('print')
input('input')
------

But the script does not print "Wrapped stdout" as prompt of input(). The script
prints the following.

-----
Wrapped stdout
input
----

Although, when sys.stdin is also wrapped, input() will use sys.stdout for prompt. For example, the following script prints "Wrapped stdout" by print() and input().

----
import io
import sys


class Writer:
    def __getattr__(self, name):
        return getattr(sys.__stdout__, name)

    def write(self, data):
        if data != '\n':
            sys.__stdout__.write('Wrapped stdout\n')

    def fileno():
        raise OSError()


class Reader:
    def __getattr__(self, name):
        return getattr(sys.__stdin__, name)

    def read(self, size):
        return sys.__stdin__.read(size)

    def fileno():
        raise OSError()


sys.stdout = Writer()
sys.stdin = Reader()

print('print')
input('input')
----

The script prints the following.

-----
Wrapped stdout
Wrapped stdout
----
msg244957 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2015-06-07 13:59
This looks like an oversight. In the implementation in [bltinmodule.c][1], if checking sys.stdout.fileno() fails, it clears the error without setting tty = 0. It's the [same in 3.5][2].

    /* We should only use (GNU) readline if Python's sys.stdin and
       sys.stdout are the same as C's stdin and stdout, because we
       need to pass it those. */
    tmp = _PyObject_CallMethodId(fin, &PyId_fileno, "");
    if (tmp == NULL) {
        PyErr_Clear();
        tty = 0;
    }
    else {
        fd = PyLong_AsLong(tmp);
        Py_DECREF(tmp);
        if (fd < 0 && PyErr_Occurred())
            return NULL;
        tty = fd == fileno(stdin) && isatty(fd);
    }
    if (tty) {
        tmp = _PyObject_CallMethodId(fout, &PyId_fileno, "");
        if (tmp == NULL)
            PyErr_Clear();
        else {
            fd = PyLong_AsLong(tmp);
            Py_DECREF(tmp);
            if (fd < 0 && PyErr_Occurred())
                return NULL;
            tty = fd == fileno(stdout) && isatty(fd);
        }
    }

[1]: https://hg.python.org/cpython/file/b4cbecbc0781/Python/bltinmodule.c#l1709
[2]: https://hg.python.org/cpython/file/7a088af5615b/Python/bltinmodule.c#l1839
msg245279 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2015-06-12 21:49
Okay, so the change is straightforward. With this change the entire test suite passes on my machine.

We'll need a test for this for the test suite, and that's far less straightforward. It seems that test.test_builtin.BuiltinTest.check_input_tty() could be a starting point. Could someone come up with a working test for this?
msg245280 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2015-06-12 21:51
Here's a patch with just the required fix (no tests).
msg252436 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-06 22:58
Patch itself looks good.
msg252455 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-07 09:28
This patch includes a test case using pty.fork() to ensure stdin is a terminal, so the bug would be triggered. It is a bit tricky, but hopefully works on Unix platforms. I have only tested it on Linux.
msg252458 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2015-10-07 10:22
LGTM. See also minor note on Rietveld.

I think it is worth to extract all input() tests to separate class.
msg252676 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-10 01:39
Okay I plan to commit a modified version where I moved the input() test methods that use pseudo terminals to a new PtyTests class. The test_input() method stays in the original class, because it shares a helper method with other tests.

I am resisting dropping that decode() step, because it makes any error message from the forked child hard to read:

AssertionError: 256 != 0 : b'quux\r\nTraceback (most recent call last):\r\n  File "/media/disk/home/proj/python/cpython/Lib/test/test_builtin.py", line 1593, in test_input_no_stdout_fileno\r\n    input("prompt")\r\nTypeError: bad argument type for built-in operation\r\n'
msg252677 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-10 02:10
New changeset 425d81d10b13 by Martin Panter in branch '3.4':
Issue #24402: Fix input() when stdout.fileno() fails; diagnosed by Eryksun
https://hg.python.org/cpython/rev/425d81d10b13

New changeset e97d940a6543 by Martin Panter in branch '3.5':
Issue #24402: Merge input() fix from 3.4 into 3.5
https://hg.python.org/cpython/rev/e97d940a6543

New changeset bcc0f8eb6797 by Martin Panter in branch 'default':
Issue #24402: Merge input() fix from 3.5
https://hg.python.org/cpython/rev/bcc0f8eb6797
msg252683 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-10 03:41
Seems my test hangs the Free BSD 9 buildbot, although Free BSD 10 apparently passed.

http://buildbot.python.org/all/builders/AMD64%20FreeBSD%209.x%203.4/builds/1128/steps/test/logs/stdio

[390/390] test_builtin
Timeout (1:00:00)!
Thread 0x0000000801407400 (most recent call first):
  File "/usr/home/buildbot/python/3.4.koobs-freebsd9/build/Lib/test/test_builtin.py", line 1585 in test_input_no_stdout_fileno
  File "/usr/home/buildbot/python/3.4.koobs-freebsd9/build/Lib/unittest/case.py", line 580 in run
  File "/usr/home/buildbot/python/3.4.koobs-freebsd9/build/Lib/unittest/case.py", line 628 in __call__
msg252686 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2015-10-10 04:52
This test also fails on Ubuntu.

[1/1] test_builtin
test test_builtin failed -- Traceback (most recent call last):
  File "/home/angwer/my-cpython/Lib/test/test_builtin.py", line 1588, in test_input_no_stdout_fileno
    self.assertEqual(status, 0, output)
AssertionError: 256 != 0 : quux
Traceback (most recent call last):
  File "/home/angwer/my-cpython/Lib/test/test_builtin.py", line 1593, in test_input_no_stdout_fileno
    input("prompt")
TypeError: bad argument type for built-in operation


1 test failed:
    test_builtin
msg252688 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-10 05:12
I wrote this on Arch Linux, so I doubt your Ubuntu failure is valid. The error you see is what I would expect if the bug were not actually fixed. I suspect you need to recompile /Python/bltinmodule.c, which contains the bug fix.

However, the test also hangs OS X buildbots. I can only guess at why it is hanging, either at the waitpid() line or the os.read() line. I am looking at refactoring my test to use the existing check_input_tty() infrastructure. It seems over the top by adding a separate pipe back to the parent process, but presumably will workaround whatever is causing the hang.
msg252689 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2015-10-10 05:28
It's my fault. I forget to recompile. Now it works.
msg252690 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2015-10-10 05:30
New changeset d8dd9015b086 by Martin Panter in branch '3.4':
Issue #24402: Factor out PtyTests.run_child() in input() tests
https://hg.python.org/cpython/rev/d8dd9015b086

New changeset 6a8f96b46dce by Martin Panter in branch '3.5':
Issue #24402: Merge potential test fix from 3.4 into 3.5
https://hg.python.org/cpython/rev/6a8f96b46dce

New changeset cb574ee7231e by Martin Panter in branch 'default':
Issue #24402: Merge potential test fix from 3.5
https://hg.python.org/cpython/rev/cb574ee7231e
msg252699 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-10-10 10:04
My second attempt seems to have avoided the hang.

For the record, the backtrace from 3.4.koobs-freebsd9 did say it was hanging at the os.waitpid() line, but I still don’t understand why.
History
Date User Action Args
2015-10-10 10:04:49martin.pantersetstatus: open -> closed

messages: + msg252699
2015-10-10 05:30:25python-devsetmessages: + msg252690
2015-10-10 05:28:45xiang.zhangsetmessages: + msg252689
2015-10-10 05:12:51martin.pantersetmessages: + msg252688
2015-10-10 04:52:10xiang.zhangsetnosy: + xiang.zhang
messages: + msg252686
2015-10-10 03:41:08martin.pantersetstatus: closed -> open

messages: + msg252683
2015-10-10 02:41:07martin.pantersetstatus: open -> closed
resolution: fixed
stage: commit review -> resolved
2015-10-10 02:10:37python-devsetnosy: + python-dev
messages: + msg252677
2015-10-10 01:39:48martin.pantersetnosy: + berker.peksag
messages: + msg252676
2015-10-08 06:00:09serhiy.storchakasetassignee: martin.panter
stage: patch review -> commit review
2015-10-07 10:22:40serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg252458
2015-10-07 09:28:43martin.pantersetfiles: + input-terminal.patch

messages: + msg252455
2015-10-06 23:04:35martin.panterlinkissue8256 dependencies
2015-10-06 22:58:42martin.pantersetnosy: + martin.panter

messages: + msg252436
stage: needs patch -> patch review
2015-06-12 21:51:13taleinatsetfiles: + issue24402.fix_only.patch
keywords: + patch
messages: + msg245280
2015-06-12 21:49:33taleinatsetnosy: + taleinat
messages: + msg245279
2015-06-08 09:47:20pitrousetstage: needs patch
versions: + Python 3.6
2015-06-08 00:11:11ned.deilysetnosy: + pitrou
2015-06-07 13:59:15eryksunsetnosy: + eryksun

messages: + msg244957
versions: + Python 3.5
2015-06-07 09:30:52Keita Kitacreate