• 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.

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

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

rebased to handle conflicts with the sub-processor work.

Rebased again.

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

@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, @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

@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.

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

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 ;)

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.

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).

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

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.

rebased to re-trigger CI.

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.

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

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.

@mdehoon is correct and we can pull the PyOS_InputHook up a level rather than add code to avoid calling it in the inner loop.

@pablogsal With the new pyrepl what is the future plan for this code?

@tacaswell Thanks, it looks good to me.

This PR is stale because it has been open for 30 days with no activity.

This is still waiting on review from a core developer. This is to my knowledge still both correct and needed.

I pushed a small fix for this in codex/pr-103931-fix-96fa029507a6. Can you take a look?

Merged that commit to this branch. Sorry, thought this was below the threshold of needing a news.

Documentation build overview

📚 cpython-previews | 🛠️ Build #34981698 | 📁 Comparing ef34c68 against main (e134a1a)

  🔍 Preview build  

+ Added

• builtins/constants.html
• builtins/exceptions.html
• builtins/functions.html
• builtins/index.html
• builtins/stdtypes.html
• builtins/threadsafety.html
• builtins/time-complexity.html
• library/asyncio-tools.html
• library/time-complexity.html
• library/tkinter.dialogs.html
• and 2 more...

± Modified

• glossary.html
• index.html
• license.html
• py-modindex.html
• c-api/arg.html
• c-api/bool.html
• c-api/buffer.html
• c-api/bytearray.html
• c-api/bytes.html
• c-api/capsule.html
• and 402 more...

@pablogsal I think this is good to go.

This PR is stale because it has been open for 90 days with no activity.

rebased on current main, still think this is ready to go!