Reproducer: ``` /var/db/repos/gentoo/dev-vcs/rcs # while true ; do ebuild rcs-5.10.1.ebuild clean unpack ; done ``` Let it run for a second or two, then ^C^C^C^C^C. It won't abort immediately and often an exit can't be induced via CTRL-C (SIGINT) at all. I was originally concerned that this was a regression with newer Bash as I hadn't noticed this behaviour before, but it seems not, after extensive discussion about a week ago in #gentoo-base. The conclusion was mostly: <tirnanog> programs that trap INT should be sure to kill themselves with INT after the trap handler has done whatever it set out to do, otherwise exactly that can happen. it's possible that ebuild(1) fails to do that. <tirnanog> in summary, sys.exit(128 + SIGNUM) is tolerable but don't do it for QUIT/INT. <@floppym> ebuild doesn't kill itself, but it does mess with the exit status. <tirnanog> that's very, very wrong. <tirnanog> it should make an exception for INT, in particular. I don't know how you write kill -INT $$ in python, but that, basically. <tirnanog> either that or don't trap INT at all. [...] <@floppym> I haven't really made my mind up on whether we should just let KeyboardInterrupt go unhandled, or set the signal handler for SIGINT to SIG_DFL. <@floppym> The former is cleaner from a python perspective, but will result in a backtrace when portage gets killed with Ctrl-C.
<dwfreed> what about a combo approach? don't mess with signal handling normally, wrap main() in try/except KeyboardInterrupt, then change SIGINT to SIG_DFL and kill(0, SIGINT) ? <dwfreed> you won't get the KeyboardInterrupt backtrace, but you'll still get exited-by-sigint status <dwfreed> for emerge, which does actual work in a ctrl+c handler, just manually throw KeyboardInterrupt when that work is done, and keep the rest as above
After looking at it a bit, I don't see any obvious reason to handle KeyboardInterrupt instead of just setting SIG_DFL and letting the kernel kill the process immediately. Is there something I'm missing?
It breaks context managers where they might be expected to work. The option of wrapping the entry point in try: except KeyboardInterrupt: signal.signal(...); os.kill(...) seems no worse to me
(In reply to Arsen Arsenovic from comment #3) It's somewhat annoying to implement since scripts like ebuild don't have a single "main" function we can wrap. I guess I can do it if someone is aware of a context manager that actually needs to run before we terminate the process.
I went ahead and implemented the "catch KeyboardInterrupt" approach in a second PR. I think I like it better.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/proj/portage.git/commit/?id=7141d7f0033bc7bf5bdf825271a0002657d4fb83 commit 7141d7f0033bc7bf5bdf825271a0002657d4fb83 Author: Mike Gilbert <floppym@gentoo.org> AuthorDate: 2022-12-27 03:57:56 +0000 Commit: Sam James <sam@gentoo.org> CommitDate: 2022-12-31 13:29:24 +0000 Rework signal handling in entry scripts Introduce a new exception SignalInterrupt which inherits from KeyboardInterrupt and adds a 'signum' member. When a signal is received, raise SignalInterrupt. At the end of the script, catch KeyboardInterrupt, and look for the signum member. Reset the signal handler to SIG_DFL, and re-raise the signal to kill the process. This ensures that the invoking shell sees that we got killed by a signal instead of calling exit. Bug: https://bugs.gentoo.org/887817 Signed-off-by: Mike Gilbert <floppym@gentoo.org> Closes: https://github.com/gentoo/portage/pull/965 Signed-off-by: Sam James <sam@gentoo.org> bin/ebuild | 781 +++++++-------- bin/ebuild-ipc.py | 505 +++++----- bin/egencache | 2473 +++++++++++++++++++++++----------------------- bin/emaint | 86 +- bin/emerge | 43 +- bin/portageq | 2854 ++++++++++++++++++++++++++--------------------------- 6 files changed, 3381 insertions(+), 3361 deletions(-)
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=1d78c5d85797399b0dbd0fe885d04b7f50fc3a60 commit 1d78c5d85797399b0dbd0fe885d04b7f50fc3a60 Author: Sam James <sam@gentoo.org> AuthorDate: 2023-01-02 05:31:32 +0000 Commit: Sam James <sam@gentoo.org> CommitDate: 2023-01-02 05:31:42 +0000 sys-apps/portage: add 3.0.43 Closes: https://bugs.gentoo.org/888487 Closes: https://bugs.gentoo.org/887817 Closes: https://bugs.gentoo.org/885909 Signed-off-by: Sam James <sam@gentoo.org> sys-apps/portage/Manifest | 1 + sys-apps/portage/portage-3.0.43.ebuild | 292 +++++++++++++++++++++++++++++++++ 2 files changed, 293 insertions(+)
