Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

trap EXIT INT does not seem to work at all with CTRL-C #6649

Closed
netvl opened this issue Feb 24, 2020 · 19 comments
Closed

trap EXIT INT does not seem to work at all with CTRL-C #6649

netvl opened this issue Feb 24, 2020 · 19 comments
Labels
bug
Milestone

Comments

@netvl
Copy link

@netvl netvl commented Feb 24, 2020

This very simple script does not print anything when I run it like fish ./test-trap and then press CTRL-C:

trap "echo After" EXIT INT
sleep 100s

If I run exactly the same script with bash and press CTRL-C, it prints After two times.

It can be reproduced with a manual function whatever --on-event fish_exit --on-signal INT as well.

Fish version: 3.0.2
Linux void 5.4.11-arch1-1 SMP PREEMPT Sun, 12 Jan 2020 12:15:27 +0000 x86_64 GNU/Linux

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Feb 27, 2020

I can reproduce, the bash script needs to be run as a script, not directly from the command line or else you engage job control. This should work in fish, I will figure out why it doesn't.

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Feb 27, 2020

In non-interactive mode, fish responds to a SIGINT-exited process by SIGINT itself. The function is enqueued but there's no place to execute it, fish is already exiting. I'll have to think about how this could work.

@scooter-dangle
Copy link

@scooter-dangle scooter-dangle commented Feb 27, 2020

I don't know if it's relevant, but I had noticed a change in behavior of the the --on-signal INT flag when upgrading to 3.1 (I think from 3.0, but I can at least verify the old, expected behavior on an installation of 2.7.1).

Previously, executing the following and then hitting ctrl-c would cause hola to be printed:

function stuffs --on-signal INT
    echo hola
end

As of 3.1.0, it doesn't.

@zanchey zanchey added the bug label Feb 28, 2020
@zanchey zanchey added this to the fish-future milestone Feb 28, 2020
@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Mar 1, 2020

Heh, yes, we enqueue the event but refuse to execute it, because we're cancelling.

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Mar 1, 2020

Second issue bisects to f8ba0ac.

ridiculousfish added a commit that referenced this issue Mar 1, 2020
f8ba0ac introduced a bug where INT handlers would themselves be
cancelled, due to the signal. Defer processing handlers until the
parser is ready to execute more fish script.

Fixes the interactive case of #6649.
@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Mar 1, 2020

It appears that non-interactive INT handlers have never worked in fish. The interactive case is a 3.1.0 regression and is fixed in bc702cc

@zanchey this would be a good addition for 3.1.1, this comment is what is fixed.

ridiculousfish added a commit that referenced this issue Mar 2, 2020
f8ba0ac introduced a bug where INT handlers would themselves be
cancelled, due to the signal. Defer processing handlers until the
parser is ready to execute more fish script.

Fixes the interactive case of #6649.
@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented Mar 2, 2020

cherry-picked to 3.1.1 as f776c4e

@zanchey zanchey removed this from the fish-future milestone Mar 13, 2020
@zanchey zanchey added this to the fish 3.1.1 milestone Mar 13, 2020
@zanchey
Copy link
Member

@zanchey zanchey commented Mar 13, 2020

Note from the original post and #6744 this appears to have been broken in 3.0.0 as well.

@zanchey zanchey removed this from the fish 3.1.1 milestone Apr 25, 2020
@zanchey zanchey added this to the fish-future milestone Apr 25, 2020
@zanchey
Copy link
Member

@zanchey zanchey commented Apr 25, 2020

Moving this out of the 3.1.1 milestone as the non-interactive case is still not fixed.

@Korkman
Copy link

@Korkman Korkman commented May 7, 2020

Note that trapping SIGINT or SIGTERM should prevent the shell from performing the default action (i.e. exit). At least there should be a way to tell fish not to exit (trap function return status?)

@bakhtiyarneyman
Copy link

@bakhtiyarneyman bakhtiyarneyman commented Mar 26, 2021

I would really like to see this fixed, so I posted a bounty on this bug.

@suzaku
Copy link

@suzaku suzaku commented Mar 26, 2021

When I use kill -s INT, I can see the output. But I see nothing with CTRL-C, so it seems like CTRL-C's not triggering SIGINT in this case.

@bakhtiyarneyman

This comment has been minimized.

@faho

This comment has been minimized.

@bakhtiyarneyman

This comment has been minimized.

@floam
Copy link
Member

@floam floam commented Oct 5, 2021

Huh, this actually works as expected if I attach a debugger and step through it.

@floam
Copy link
Member

@floam floam commented Oct 5, 2021

Er, you need to do a pro hand -p true -s false SIGINT in lldb to get the signal passed like normal.

@ridiculousfish ridiculousfish removed this from the fish-future milestone May 29, 2022
@ridiculousfish ridiculousfish added this to the fish 3.5.0 milestone May 29, 2022
@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented May 29, 2022

Fixed in cf2ca56

@ridiculousfish
Copy link
Member

@ridiculousfish ridiculousfish commented May 29, 2022

@bakhtiyarneyman I appreciate your offering a bounty but it is not necessary, please keep your money or donate it to a worthy cause. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug
Projects
None yet
Development

No branches or pull requests

9 participants