mitnk / cicada

An old-school bash-like Unix shell written in Rust
https://hugo.wang/cicada/
MIT License
981 stars 50 forks source link

Cicada not quitting on SIGINT #2

Closed badboy closed 6 years ago

badboy commented 7 years ago

First, thanks! Looks like a really cool project.

Cicada does not quit correctly when sent a SIGINT signal. If sent externally, simply nothing is happening. If sent through typing Ctrl-C directly in the shell, my terminal closes output, but cicada keeps running. The terminal is then spammed with readline error: Error { repr: Os { code: 5, message: "Input/output error" } }, due to the closed stdout. At this point cicada consumes 100% of the CPU.

You should handle external signals and correctly quit the application. (I also expected to be able to quit the shell using Ctrl-D aka sending EOF, but this does nothing

mitnk commented 7 years ago

Thanks @badboy, first. cicaca can, you know, always exit with a exit command.

Cicada does not quit correctly when sent a SIGINT signal.

I'll try and see.

I also expected to be able to quit the shell using Ctrl-D

Cicada can provide this feature but would disable it by default - since Ctrl-D to exit the whole shell sounds dangerous to me, especially as a login shell.

mitnk commented 7 years ago

Hi @badboy, Cicada not quitting for SIGINT is on purpose. If you want it to exit, you could use SIGQUIT or SIGTERM. I'm closing this one, and feel free to reopen if needed.

badboy commented 7 years ago

Do you at least now handle a closed output and exit the program without keeping to write error messages?

mitnk commented 7 years ago

sent through typing Ctrl-C directly in the shell, my terminal closes output,

I seems cannot reproduce this. Which OS/Terminal are you using?

Have you tried starting a new bash session in your terminal, and typing Ctrl-C, what is the behaviour of this bash process?

badboy commented 7 years ago
[~]% bash
[jer@damogran ~]$ ^C
[jer@damogran ~]$ ^C
[jer@damogran ~]$ 

so yeah, not ending the shell is the right way (though Ctrl+D still works ;)). I can test if I can reproduce the other issue in Cicada later

mitnk commented 6 years ago

Ctrl-D exits cicada is the default behavior now (see #8), though I'm going to add an extra ENV to change it.

badboy commented 6 years ago

Cool. Will check if any of this issue is still valid or close the issue.

badboy commented 6 years ago

Looks all good and shiny, no crashes, no unhandled closed output and Ctrl-D works as expected (and configured). Thanks!