Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ambv merged 18 commits intopython:mainfromyihong0618:hy/fix_can_not_strace
Sep 16, 2025

Conversation

@yihong0618
Copy link
Contributor

@yihong0618yihong0618 commentedAug 25, 2025
edited
Loading

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.

hyongtao-code and whitescent reacted with heart emoji
Signed-off-by: yihong0618 <zouzou0208@gmail.com>Co-authored-by: dura0ok <slpmcf@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Copy link
Member

@ZeroIntensityZeroIntensity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Please try adding a test case.

yihong0618 reacted with heart emoji
@yihong0618
Copy link
ContributorAuthor

Please try adding a test case.

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

@ZeroIntensity
Copy link
Member

Why not just run withstrace itself withsubprocess?

@yihong0618
Copy link
ContributorAuthor

Why not just run withstrace itself withsubprocess?

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

@ZeroIntensity
Copy link
Member

Yeah, we just skip the test on those platforms.

@yihong0618
Copy link
ContributorAuthor

Yeah, we just skip the test on those platforms.

just found cpython already havestrace maybe

will try thanks for the info

Lib/test/support/strace_helper.py11:_strace_binary="/usr/bin/strace"27:strace_returncode:int30:"""The event messages generated by strace. This is very similar to the31:    stderr strace produces with returncode marker section removed."""40:stringswhichwouldmixintothestraceoutput."""97:def strace_python(code, strace_flags, check=True):98:    """Runstraceandreturnthetrace.100:Setsstrace_returncodeandpython_returncodeto`-1`onerror."""105:            strace_returncode=-1,111:    # Run strace, and get out the raw text116:            __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"]

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618
Copy link
ContributorAuthor

tests case added

in main

yihong@ubuntu:~/cpython$ ./python -m unittest test.test_pyrepl.test_unix_console.TestUnixConsoleEIOHandlingFF======================================================================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 effecttermios.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 effecttermios.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.002sFAILED (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
Copy link
Contributor

👍 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 wrapstermios.tcsetattr already) to contain the try/except. Searching through_pyrepl only_pyrepl.unix_console uses_prepl.fancy_termios (and the one othertermios.tcsetattr is in a finally where ignoring EIO would probably be reasonable as well)

yihong0618 and ZeroIntensity reacted with heart emoji

@yihong0618
Copy link
ContributorAuthor

👍 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 wrapstermios.tcsetattr already) to contain the try/except. Searching through_pyrepl only_pyrepl.unix_console uses_prepl.fancy_termios (and the one othertermios.tcsetattr is in a finally where ignoring EIO would probably be reasonable as well)

thanks let me test and wait for the reviewer check~

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618
Copy link
ContributorAuthor

yihong0618 commentedAug 27, 2025
edited
Loading

@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
Copy link
Contributor

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

ZeroIntensity reacted with thumbs up emoji

@yihong0618
Copy link
ContributorAuthor

@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
Copy link
ContributorAuthor

@ZeroIntensity test added strace with EIO

Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618
Copy link
ContributorAuthor

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
Copy link
Member

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
Copy link
ContributorAuthor

yihong0618 commentedSep 8, 2025
edited
Loading

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

@yihong0618yihong0618 marked this pull request as draftSeptember 8, 2025 11:07
yihong0618and others added2 commitsSeptember 12, 2025 17:06
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
@yihong0618yihong0618 marked this pull request as ready for reviewSeptember 12, 2025 09:24
Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

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

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
@picnixzpicnixz dismissed theirstale reviewSeptember 16, 2025 10:07

Changes were applied. Only final nitpicks remain.

@ambvambv added needs backport to 3.13bugs and security fixes needs backport to 3.14bugs and security fixes labelsSep 16, 2025
@ambvambv merged commitb9dbf6a intopython:mainSep 16, 2025
57 checks passed
@miss-islington-app
Copy link

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 pushed a commit to miss-islington/cpython that referenced this pull requestSep 16, 2025
…pythonGH-138133)(cherry picked from commitb9dbf6a)Co-authored-by: yihong <zouzou0208@gmail.com>Signed-off-by: yihong0618 <zouzou0208@gmail.com>Co-authored-by: dura0ok <slpmcf@gmail.com>Co-authored-by: graymon <greyschwinger@gmail.com>Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>Co-authored-by: Łukasz Langa <lukasz@langa.pl>
@miss-islington-app
Copy link

Sorry,@yihong0618 and@ambv, I could not cleanly backport this to3.13 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker b9dbf6acb34fd407d52899a6c154a1c57c9a424b 3.13

@bedevere-app
Copy link

GH-138973 is a backport of this pull request to the3.14 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.14bugs and security fixes labelSep 16, 2025
ambv added a commit to ambv/cpython that referenced this pull requestSep 16, 2025
…pythonGH-138133)Signed-off-by: yihong0618 <zouzou0208@gmail.com>Co-authored-by: dura0ok <slpmcf@gmail.com>Co-authored-by: graymon <greyschwinger@gmail.com>Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>Co-authored-by: Łukasz Langa <lukasz@langa.pl>(cherry picked from commitb9dbf6a)
ambv added a commit to ambv/cpython that referenced this pull requestSep 16, 2025
… strace (pythonGH-138133)(cherry picked from commitb9dbf6a)Co-authored-by: yihong <zouzou0208@gmail.com>Signed-off-by: yihong0618 <zouzou0208@gmail.com>Co-authored-by: dura0ok <slpmcf@gmail.com>Co-authored-by: graymon <greyschwinger@gmail.com>Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>Co-authored-by: Łukasz Langa <lukasz@langa.pl>
pablogsal pushed a commit that referenced this pull requestSep 16, 2025
GH-138133) (#138973)gh-135329: prevent infinite traceback loop on Ctrl-C  for strace (GH-138133)(cherry picked from commitb9dbf6a)Signed-off-by: yihong0618 <zouzou0208@gmail.com>Co-authored-by: yihong <zouzou0208@gmail.com>Co-authored-by: dura0ok <slpmcf@gmail.com>Co-authored-by: graymon <greyschwinger@gmail.com>Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>Co-authored-by: Łukasz Langa <lukasz@langa.pl>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@picnixzpicnixzpicnixz left review comments

@pablogsalpablogsalAwaiting requested review from pablogsalpablogsal is a code owner

@lysnikolaoulysnikolaouAwaiting requested review from lysnikolaoulysnikolaou is a code owner

@ambvambvAwaiting requested review from ambvambv is a code owner

@ZeroIntensityZeroIntensityAwaiting requested review from ZeroIntensity

@cmaloneycmaloneyAwaiting requested review from cmaloney

Assignees

@ambvambv

Labels

needs backport to 3.13bugs and security fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@yihong0618@ZeroIntensity@cmaloney@picnixz@ambv

[8]ページ先頭

©2009-2025 Movatter.jp