gh-130168: pyrepl can messup and poll is not thread safe

[yihong0618]

This fix issue 130168
but for this one it contains two issues:

the first one is for poll here is not thread safe
more can check. ##53111

and the second one is for the prepare and restore the error message will mess up.

and for the default pyrepl >= 3.13 the input is wrote in python different from the old one

this patch make the error same which is can't re-enter readline like python3.12

before this patch:

after this patch:
which not mess up anything

cc @gaogaotiantian can you help to check?

Thank you very much.

• Issue: pyrepl messed up terminal if a signal handler expects stdin #130168

[picnixz]

Hum, now tests fail but I don't know why. Is there another place where we use the internal state?

[yihong0618]

Hum, now tests fail but I don't know why. Is there another place where we use the internal state?

same like #124030

[picnixz]

Yeah but https://github.com/python/cpython/actions/runs/17528155338/job/49781429653?pr=138617 worked and it was before my suggestions. Ok, remove the addCleanup part, this may be the culprit and do it normally (that is as you did before)

[yihong0618]

Yeah but https://github.com/python/cpython/actions/runs/17528155338/job/49781429653?pr=138617 worked and it was before my suggestions. Ok, remove the addCleanup part, this may be the culprit and do it normally (that is as you did before)

Done will check the test

[picnixz]

Mmh, so apparently the issue is indeed with the cleanup. Though I don't understand why.... because cleanups are called in LIFO.

[picnixz]

When I meant "remove it", I meant remove the addCleanup, but keep console._polling_thread = None; console.restore() at the end of the test function.

[yihong0618]

When I meant "remove it", I meant remove the addCleanup, but keep console._polling_thread = None; console.restore() at the end of the test function.

sorry for forget that, now move it back

[yihong0618]

Mmh, so apparently the issue is indeed with the cleanup. Though I don't understand why.... because cleanups are called in LIFO.

do we need to open an issue to track this?

[yihong0618]

This patch also make #129614 the same behavior as 3.12

[picnixz]

do we need to open an issue to track this?

Maybe, but I'll need to check with a smaller reproducer.

[yihong0618]

Mmh, so apparently the issue is indeed with the cleanup. Though I don't understand why.... because cleanups are called in LIFO.

@picnixz after some dig found why here

the issue is that we mock the tcgetattr the arg in the patch

@patch(
"termios.tcgetattr",
lambda _: [
27394,
3,
19200,
536872399,
38400,
38400,
[
b"\x04",
b"\xff",
b"\xff",
b"\x7f",
b"\x17",
b"\x15",
b"\x12",
b"\x00",
b"\x03",
b"\x1c",
b"\x1a",
b"\x19",
b"\x11",
b"\x13",
b"\x16",
b"\x0f",
b"\x01",
b"\x00",
b"\x14",
b"\x00",
],
],
)
@patch("termios.tcsetattr", lambda a, b, c: None)

it is fine but in some system like ubuntu arm 24
the length of it is 32

but

this length is 20

 [
b"\x04",
b"\xff",
b"\xff",
b"\x7f",
b"\x17",
b"\x15",
b"\x12",
b"\x00",
b"\x03",
b"\x1c",
b"\x1a",
b"\x19",
b"\x11",
b"\x13",
b"\x16",
b"\x0f",
b"\x01",
b"\x00",
b"\x14",
b"\x00",
],

you can use this script to check

importsysimporttermiossys.path.insert(0, "./Lib")
from_pyrepl.fancy_termiosimporttcgetattr, tcsetattrdefmain():
try:
state=tcgetattr(0)
print("Real tcgetattr works, cc length:", len(state.cc))
tcsetattr(0, termios.TCSADRAIN, state)
print("Real tcsetattr works")
exceptExceptionase:
print("Error:", e)
if__name__=="__main__":
main()

if we use self.addclean ... it calls the patch and the length is different

[hyi@rocky cpython]$ uname -a
Linux rocky 6.15.11-orbstack-00539-g9885ebd8e3f4 #1 SMP PREEMPT Fri Aug 22 08:24:56 UTC 2025 aarch64 aarch64 aarch64 GNU/Linux
[hyi@rocky cpython]$ ./python test.py Real tcgetattr works, cc length: 32
Real tcsetattr works
➜ cpython git:(hy/close_new_repl_error_mess) ✗ uname -a
Darwin hyideMacBook-Pro.local 24.5.0 Darwin Kernel Version 24.5.0: Tue Apr 22 19:54:33 PDT 2025; root:xnu-11417.121.6~2/RELEASE_ARM64_T8122 arm64
➜ cpython git:(hy/close_new_repl_error_mess) ✗ ./python.exe test.py Real tcgetattr works, cc length: 20
Real tcsetattr works

so I think we add console.restore() in the last is fine

[yihong0618]

merge and fix the conflict

[yihong0618]

interesting fail will try to figure out why

---

fixed

[chris-eibl]

I gave this PR a try on native Ubuntu 24.04.3 LTS, and it now shows RuntimeError: can't re-enter readline, but the output is still messed up.

Details

[chris-eibl]

it now shows RuntimeError: can't re-enter readline

I understand this is to be in sync what the old REPL showed, but isn't that misleading, because here readline isn't involved at all?

[picnixz]

I understand this is to be in sync what the old REPL showed, but isn't that misleading, because here readline isn't involved at all?

It is, through reader.readline(). But it's not the readline you think of. But we can think of a better message as it can be confusing indeed.

I gave this PR a try on native Ubuntu 24.04.3 LTS, and it now shows RuntimeError: can't re-enter readline, but the output is still messed up.

Mmh, then the fix should be different probably. The OP's output seems to be on darwin which is Unix-based as well. Note that unix_console defines a local poll class that is re-implemented in case select.poll is unavailable.

In addition, on Apple terminals, line wrap is disabled so maybe something is different here as well?

[chris-eibl]

Just FTR, confirming that Windows isn't affected (since as said like on Linux no readline is involved and the WaitForSingleObject seems to be re-entrant.

Legacy console:

Virtual terminal:

The prompt is overridden in this case, but this will be solved by #138732.

[yihong0618]

I gave this PR a try on native Ubuntu 24.04.3 LTS, and it now shows RuntimeError: can't re-enter readline, but the output is still messed up.

Details

Interesting will test it

[chris-eibl]

It is, through reader.readline(). But it's not the readline you think of. But we can think of a better message as it can be confusing indeed.

Oh, I know :) reader.readline() shows in the traceback.

I was referring to

this patch make the error same which is can't re-enter readline like python3.12

Why not just fail with "concurrent poll" - why mimic the 3.12 error message here?
I mean it is not totally wrong, but might be confusing ...

[yihong0618]

It is, through reader.readline(). But it's not the readline you think of. But we can think of a better message as it can be confusing indeed.

Oh, I know :) reader.readline() shows in the traceback.

I was referring to

this patch make the error same which is can't re-enter readline like python3.12

Why not just fail with "concurrent poll" - why mimic the 3.12 error message here?

I mean it is not totally wrong, but might be confusing ...

sorry for the confusion next time will try to use LLM or something else to make a better desc.
sorry for that

[yihong0618]

I gave this PR a try on native Ubuntu 24.04.3 LTS, and it now shows RuntimeError: can't re-enter readline, but the output is still messed up.

Details

yes it is mess up
merge main break the fix ....
will figure out why

[yihong0618]

@chris-eibl now fixed the mess up can you help to check?

[chris-eibl]

Works now for me on native Ubuntu 24.04.3 LTS.

[yihong0618]

Works now for me on native Ubuntu 24.04.3 LTS.

thank you for confirm the reason is that merge conflict~

[chris-eibl]

Still unsure about the RuntimeError: can't re-enter readline.

IMHO the PR would be simpler without trying to mimic it, but that's the the pyrepl maintainers decision, anyway.

[yihong0618]

Still unsure about the RuntimeError: can't re-enter readline.

IMHO the PR would be simpler without trying to mimic it, but that's the the pyrepl maintainers decision, anyway.

the reason I choose that is make the runtime error message as basic repl

[picnixz]

Yeah, maybe we can change the message. I don't have a suggestion now but I'd like REPL maintainers to first check this PR.