GH-103929: handle long input lines with PyOS_InputHook

[tacaswell]

If:

• stdin is in line buffer mode
• the readline module is not loaded
• not on windows
• a GUI toolkit has installd PyOS_InputHook

Then, if the user enters lines longer than 98 characters into input:

• user calls input(...)
• user types/pastes a line longer than 98 characters + 1 newline
• PyOS_InputHook returns
• the first 99 characters are returned
• the last character is not a new line so we go back to my_fgets
• the PyOS_InputHook blocks because despite there being value in the buffer, stdin does not flag itself as ready to be read again
• user hits enter again and to finish reading the input
• the extra new line comes out the next time the user calls input

This fixes this bug by passing the currently read number of characters to my_fgets to skip the input hook when reading out a long buffer.

closes #103929

---

Qustions:

• I do not know how to test this other than manually
• It seems to work when stdin is unbuffered, but I do not fully understand why
• Is the edit to the docs of PyOS_InputHook correct?

• Issue: non-readline PyOS_StdioReadline when used with PyOS_InputHook fails is buggy with long input #103929

[mdboom]

• I do not know how to test this other than manually

The usual way to test a C API call directly is add a method to the testcapi module. It seems like you could test this through a call to PyOS_StdioReadline, but it's not exactly trivial to set up the FILE objects and buffer them with data etc. I don't see any other tests in testcapi doing anything like that.

[mdboom]

@brandtbucher: Maybe you have enough context to review this since you've been in readline-adjacent things lately?

[brandtbucher]

I'll take a look today next week. Thanks for the PR!

[tacaswell]

rebased to handle conflicts with the sub-processor work.

[tacaswell]

Rebased again.

[jdtsmith]

@brandtbucher did you ever get a chance to review this one?

[mdboom]

@pablogsal, @ambv: If I understand correctly, users are more likely to hit this bug in 3.14 since the new REPL doesn't use readline. Should we try to get this into the next 3.14 beta?

[pablogsal]

@pablogsal, @ambv: If I understand correctly, users are more likely to hit this bug in 3.14 since the new REPL doesn't use readline. Should we try to get this into the next 3.14 beta?

This fix doesn't apply to the new REPL, so I don't think how this PR could help here

[mdboom]

@pablogsal, @ambv: If I understand correctly, users are more likely to hit this bug in 3.14 since the new REPL doesn't use readline. Should we try to get this into the next 3.14 beta?

This fix doesn't apply to the new REPL, so I don't think how this PR could help here

Got it. My bad.

[jdtsmith]

@pablogsal Does that mean the new REPL should/will not have this 100 char limit while PyOS_InputHook is installed?

[tacaswell]

The bug that is fixed is in the readline-replacement implementation that Python carries, not in the input hooks, so if there are bugs in the new repl with long lines, they will be different bugs ;)

[jdtsmith]

I'll take that as a solid maybe ;). Will report if I find similar long-line bugs with active input hooks. Update: re-reading, it seems you mean the bug remains, but that the new REPL has worked around it in another way. Those of us using input-based REPL's the still need this fix.

[mdehoon]

This bug shows up not only with input() but also with interactive Python when typing commands at the Python prompt.

For example, running the commands in this script one-by-one will freeze Python after long_string until the user hits enter one more time:

>>> import tkinter
>>> t = tkinter.Tk()
>>> short_string = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
>>> long_string = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"

>>> >>> ^D

(as @tacaswell mentioned, the bug only appears on Python without readline).

[jdtsmith]

It would be nice to get this reviewed and merged. For now the only workaround is import readline, which may be undesirable.

[mdehoon]

It seems to work when stdin is unbuffered, but I do not fully understand why

When stdin is unbuffered, then fgets reads directly from stdin and leaves any characters in stdin that do not fit in the fgets buffer. PyOS_InputHook will then see those remaining characters, and return.

When stdin is buffered, then fgets reads the data from stdin into an internal buffer. This internal buffer is likely bigger than even the long input line, so it will read the full line into its internal buffer, leaving nothing in stdin. PyOS_InputHook then doesn't find anything in stdin, and will block waiting for new input.

[tacaswell]

rebased to re-trigger CI.

[mdehoon]

Is the edit to the docs of PyOS_InputHook correct?

(meaning this sentence: "This function [PyOS_InputHook] should block until stdin is readable.").

I have not seen it described in the documentation, but this is how PyOS_InputHook is used in practice (e.g. in the EventHook function in _tkinter.c, and in other GUI toolkits such as PyGTK). I think it is the only reasonable way to use PyOS_InputHook, and it would be good to have its usage described somewhere in the documentation.

But, if so, then the current PR is not ideal. Once we start calling fgets, then we have already decided that there is input available on stdin, and there is no point in calling PyOS_InputHook again. Then I would think that PyOS_InputHook needs to be called only once, just before we get into the do { ... } while (p[n-1] != '\n'); loop, and it does not need to be called from my_fgets.

[tacaswell]

I think we need the inputhook inside of that do...while loop as if the input hook returns early (like some flavors on windows do).

Maybe it should be rephrased to:

This function [PyOS_InputHook] should block until stdin is readable but may return early

This lets you run a loop like "run UI for 5ms, check stid, run UI for 5ms ...". This bit of code is here due to https://github.com/python/cpython/pull/7978/files which I put in with guidance from @zooba .

[mdehoon]

This lets you run a loop like "run UI for 5ms, check stid, run UI for 5ms ...".

This loop happens inside of the PyOS_InputHook function. See the EventHook function in Modules\_tkinter.c, which boils down to

    while (!stdin_ready) {
        int result = Tcl_DoOneEvent(TCL_DONT_WAIT);
        if (result == 0)
            Sleep(Tkinter_busywaitinterval);
    }

There is no need to have such a second loop on top of it outside of PyOS_InputHook.

See also pull request #140147 which gets rid of the Windows-specific code in EventHook.

[tacaswell]

But does it happen inside the function for all function currently in the wild? I have a recent-ish survey here: #119843 (comment)

[mdehoon]

But does it happen inside the function for all function currently in the wild? I have a recent-ish survey here: #119843 (comment)

In the main packages (matplotlib, PyQt5, PyQt6, PySide, and PyGTK), PyOS_InputHook points to a function that returns once data is available on stdin.

PyGObject (which superseded PyGTK) does not use PyOS_InputHook.

For wxPython: there was a discussion about it 16 years ago (see https://discuss.wxpython.org/t/interactive-usage-of-wxpython/32128/22, where I was making the same comments). But I did not find any (recent or old) version of wxPython that actually uses PyOS_InputHook, so I guess it was never implemented.

Now there may be some module in the wild that assumes that PyOS_InputHook is called repeatedly. I didn't find such a module, but if it exists, its event loop will freeze with Python without readline support.