gh-135329: prevent infinite traceback loop on Ctrl-C for strace

[yihong0618]

This patch fix strace can not ctrl+c error

and follow the work so add co-author dura0ok

but that fix is not enough, there is a another problem in the queue
we need to consider

cc @picnixz @cmaloney you can try this patch.

• Issue: Running python REPL under another program + Ctrl-C results in an exception traceback #135329

[ZeroIntensity]

Please try adding a test case.

[yihong0618]

Please try adding a test case.

tried but kind of hard...
It's kind of hard to make some env like strace

[ZeroIntensity]

Why not just run with strace itself with subprocess?

[yihong0618]

Why not just run with strace itself with subprocess?

ok but some env do not have it, is that OK?

[ZeroIntensity]

Yeah, we just skip the test on those platforms.

[yihong0618]

Yeah, we just skip the test on those platforms.

just found cpython already have strace maybe

will try thanks for the info

Lib/test/support/strace_helper.py
11:_strace_binary = "/usr/bin/strace"
27:    strace_returncode: int
30:    """The event messages generated by strace. This is very similar to the
31:    stderr strace produces with returncode marker section removed."""
40:        strings which would mix into the strace output."""
97:def strace_python(code, strace_flags, check=True):
98:    """Run strace and return the trace.
100:    Sets strace_returncode and python_returncode to `-1` on error."""
105:            strace_returncode=-1,
111:    # Run strace, and get out the raw text
116:            __run_using_command=[_strace_binary] + strace_flags,
128:        return _make_error("Expected strace events and exit code line",
140:    return StraceResult(strace_returncode=res.rc,
147:def get_events(code, strace_flags, prelude, cleanup):
162:    trace = strace_python(to_run, strace_flags)
167:def get_syscalls(code, strace_flags, prelude="", cleanup="",
170:    events = get_events(code, strace_flags, prelude=prelude, cleanup=cleanup)
180:def _can_strace():
181:    res = strace_python("import sys; sys.exit(0)",
182:                        # --trace option needs strace 5.5 (gh-133741)
185:    if res.strace_returncode == 0 and res.python_returncode == 0:
191:def requires_strace():
193:        return unittest.skip("Linux only, requires strace.")
203:        return unittest.skip("LeakSanitizer does not work under ptrace (strace, gdb, etc)")
205:    return unittest.skipUnless(_can_strace(), "Requires working strace")
208:__all__ = ["filter_memory", "get_events", "get_syscalls", "requires_strace",
209:           "strace_python", "StraceEvent", "StraceResult"]

[yihong0618]

tests case added

in main

yihong@ubuntu:~/cpython$ ./python -m unittest test.test_pyrepl.test_unix_console.TestUnixConsoleEIOHandling
FF
======================================================================
FAIL: test_eio_error_handling_in_prepare (test.test_pyrepl.test_unix_console.TestUnixConsoleEIOHandling.test_eio_error_handling_in_prepare)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/yihong/cpython/Lib/test/test_pyrepl/test_unix_console.py", line 330, in test_eio_error_handling_in_prepare
    console.prepare()
    ~~~~~~~~~~~~~~~^^
  File "/home/yihong/cpython/Lib/_pyrepl/unix_console.py", line 339, in prepare
    tcsetattr(self.input_fd, termios.TCSADRAIN, raw)
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yihong/cpython/Lib/unittest/mock.py", line 1175, in __call__
    return self._mock_call(*args, **kwargs)
           ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/home/yihong/cpython/Lib/unittest/mock.py", line 1179, in _mock_call
    return self._execute_mock_call(*args, **kwargs)
           ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/home/yihong/cpython/Lib/unittest/mock.py", line 1234, in _execute_mock_call
    raise effect
termios.error: (5, 'Input/output error')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/yihong/cpython/Lib/unittest/mock.py", line 1432, in patched
    return func(*newargs, **newkeywargs)
  File "/home/yihong/cpython/Lib/test/test_pyrepl/test_unix_console.py", line 333, in test_eio_error_handling_in_prepare
    self.fail("EIO error should have been handled gracefully in prepare()")
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: EIO error should have been handled gracefully in prepare()

======================================================================
FAIL: test_eio_error_handling_in_restore (test.test_pyrepl.test_unix_console.TestUnixConsoleEIOHandling.test_eio_error_handling_in_restore)
Test that EIO errors during restore() are handled gracefully.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/yihong/cpython/Lib/test/test_pyrepl/test_unix_console.py", line 360, in test_eio_error_handling_in_restore
    console.restore()
    ~~~~~~~~~~~~~~~^^
  File "/home/yihong/cpython/Lib/_pyrepl/unix_console.py", line 371, in restore
    tcsetattr(self.input_fd, termios.TCSADRAIN, self.__svtermstate)
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/yihong/cpython/Lib/unittest/mock.py", line 1175, in __call__
    return self._mock_call(*args, **kwargs)
           ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/home/yihong/cpython/Lib/unittest/mock.py", line 1179, in _mock_call
    return self._execute_mock_call(*args, **kwargs)
           ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/home/yihong/cpython/Lib/unittest/mock.py", line 1234, in _execute_mock_call
    raise effect
termios.error: (5, 'Input/output error')

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/yihong/cpython/Lib/unittest/mock.py", line 1432, in patched
    return func(*newargs, **newkeywargs)
  File "/home/yihong/cpython/Lib/test/test_pyrepl/test_unix_console.py", line 363, in test_eio_error_handling_in_restore
    self.fail("EIO error should have been handled gracefully in restore()")
    ~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: EIO error should have been handled gracefully in restore()

----------------------------------------------------------------------
Ran 2 tests in 0.002s

FAILED (failures=2)

after this patch

yihong@ubuntu:~/cpython$ ./python -m unittest test.test_pyrepl.test_unix_console.TestUnixConsoleEIOHandling
..

Ran 2 tests in 0.001s

OK

[cmaloney]

👍 tested PR locally and resolves the issues I had using Ctrl-C when Python is running under strace.

thought: This wraps all the _pyrepl.fancy_termios.tcsetattr calls with "ignore errors.EIO"; it might make sense to just update _prepl.fancy_termios.tcsetattr (which wraps termios.tcsetattr already) to contain the try/except. Searching through _pyrepl only _pyrepl.unix_console uses _prepl.fancy_termios (and the one other termios.tcsetattr is in a finally where ignoring EIO would probably be reasonable as well)

[yihong0618]

👍 tested PR locally and resolves the issues I had using Ctrl-C when Python is running under strace.

thought: This wraps all the _pyrepl.fancy_termios.tcsetattr calls with "ignore errors.EIO"; it might make sense to just update _prepl.fancy_termios.tcsetattr (which wraps termios.tcsetattr already) to contain the try/except. Searching through _pyrepl only _pyrepl.unix_console uses _prepl.fancy_termios (and the one other termios.tcsetattr is in a finally where ignoring EIO would probably be reasonable as well)

thanks let me test and wait for the reviewer check~

[yihong0618]

@ZeroIntensity @cmaloney Hi we can not add test for subprocess for this issue
because background run can not trigger eio error...

and FYI

this patch also fix

the code in the comments

#135329 (comment)

[cmaloney]

@ZeroIntensity @cmaloney Hi we can not add test for subprocess for this issue because background run can not trigger eio error...

This doesn't actually follow for me. pyrepl does have "like its at a terminal" tests (ex: https://github.com/python/cpython/blob/main/Lib/test/test_pyrepl/test_pyrepl.py#L40-L149). That we're adding another wrapping process (strace), and then that wrapping process is killed first (leaving the subprocess an orphan) seems like something that should be possible to replicate.

[yihong0618]

@ZeroIntensity @cmaloney Hi we can not add test for subprocess for this issue because background run can not trigger eio error...

This doesn't actually follow for me. pyrepl does have "like its at a terminal" tests (ex: https://github.com/python/cpython/blob/main/Lib/test/test_pyrepl/test_pyrepl.py#L40-L149). That we're adding another wrapping process (strace), and then that wrapping process is killed first (leaving the subprocess an orphan) seems like something that should be possible to replicate.

not so simple you can refer this comment
#135329 (comment)

[yihong0618]

@ZeroIntensity test added strace with EIO

[yihong0618]

strace ./python behavior

If you want to mimic that behavior, why can't you just use strace directly?

no strace directly also can not as I left comments before
because in subprocess EIO will not raise. I tried quite sometime.

maybe I am wrong, will try again

[picnixz]

After reading through the issue I understand what you want to do. But I think we should extract that script into a separate file because it's too as a string-only script. Also, why can't we simply use the reproducer given by #135329 (comment) and perform the manual kills? is it because the test process would still be the parent process and so it's not possible?

[yihong0618]

After reading through the issue I understand what you want to do. But I think we should extract that script into a separate file because it's too as a string-only script. Also, why can't we simply use the reproducer given by #135329 (comment) and perform the manual kills? is it because the test process would still be the parent process and so it's not possible?

yes we can not that script is not fully with EIO

---

will fix it later and try to make it better if I can. Thank you very much for the review

[picnixz]

I would suggest using better "messages" to print the success. Something like: SUCCESS or FAILURE[errno] should be sufficient and match return code.

Changes were applied. Only final nitpicks remain.

[miss-islington-app]

Thanks @yihong0618 for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

[miss-islington-app]

Sorry, @yihong0618 and @ambv, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker b9dbf6acb34fd407d52899a6c154a1c57c9a424b 3.13

[bedevere-app]

GH-138973 is a backport of this pull request to the 3.14 branch.