Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork33.3k
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
Conversation
Signed-off-by: yihong0618 <zouzou0208@gmail.com>Co-authored-by: dura0ok <slpmcf@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
ZeroIntensity left a comment
There was a problem hiding this 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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Misc/NEWS.d/next/Library/2025-08-25-18-06-04.gh-issue-138133.Zh9rGo.rst OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
yihong0618 commentedAug 25, 2025
tried but kind of hard... |
ZeroIntensity commentedAug 25, 2025
Why not just run with |
yihong0618 commentedAug 25, 2025
ok but some env do not have it, is that OK? |
ZeroIntensity commentedAug 25, 2025
Yeah, we just skip the test on those platforms. |
yihong0618 commentedAug 25, 2025
just found cpython already have 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>
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
yihong0618 commentedAug 25, 2025
tests case added in main after this patch yihong@ubuntu:~/cpython$ ./python -m unittest test.test_pyrepl.test_unix_console.TestUnixConsoleEIOHandling |
cmaloney commentedAug 25, 2025
👍 tested PR locally and resolves the issues I had using Ctrl-C when Python is running under strace. thought: This wraps all the |
yihong0618 commentedAug 26, 2025
thanks let me test and wait for the reviewer check~ |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
yihong0618 commentedAug 27, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@ZeroIntensity@cmaloney Hi we can not add test for subprocess for this issue and FYI this patch also fix the code in the comments |
cmaloney commentedAug 27, 2025
This doesn't actually follow for me. |
yihong0618 commentedAug 29, 2025
not so simple you can refer this comment |
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
…/cpython into hy/fix_can_not_strace
yihong0618 commentedSep 1, 2025
@ZeroIntensity test added strace with EIO |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
picnixz commentedSep 8, 2025
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 commentedSep 8, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 |
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
Signed-off-by: yihong0618 <zouzou0208@gmail.com>
picnixz left a comment
There was a problem hiding this 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.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Changes were applied. Only final nitpicks remain.
b9dbf6a intopython:mainUh oh!
There was an error while loading.Please reload this page.
Thanks@yihong0618 for the PR, and@ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. |
…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>
Sorry,@yihong0618 and@ambv, I could not cleanly backport this to |
GH-138973 is a backport of this pull request to the3.14 branch. |
…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)
… 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>
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>
Uh oh!
There was an error while loading.Please reload this page.
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.