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

Keyboard interrupt (SIGINT) during a shell.RunInteractiveShell call from a plugin stops the editor itself #2612

Closed
Andriamanitra opened this issue Oct 31, 2022 · 2 comments · Fixed by #3357

Comments

@Andriamanitra
Copy link
Contributor

Description of the problem or steps to reproduce

  1. Make a plugin that uses shell.RunInteractiveShell() in ~/.config/micro/plug/exectest/exectest.lua:
local micro = import("micro")
local config = import("micro/config")
local shell = import("micro/shell")

function init()
	config.MakeCommand("exec", exec, config.NoComplete)
end

function exec()
	shell.RunInteractiveShell("sh -c 'echo press ctrl+c && sleep 10'", true, false)
end
  1. Run the plugin (ctrl+e and type exec)
  2. Press ctrl+c during execution

I would expect this to stop the interactive shell and return to the file I was editing. What happens instead is it kills the entire micro-editor process. It looks like this is also not what the code for shell.RunInteractiveShell was intended to do:

// This is a trap for Ctrl-C so that it doesn't kill micro
// Instead we trap Ctrl-C to kill the program we're running
c := make(chan os.Signal, 1)
signal.Notify(c, os.Interrupt)
go func() {
for range c {
cmd.Process.Kill()
}
}()

Version information

OS: Linux (OpenSUSE Tumbleweed)
Version: 0.0.0-unknown
Commit hash: c8c7ad5
Compiled on October 31, 2022

@Andriamanitra
Copy link
Contributor Author

I spent some time trying to fix this and realized the issue stems from the sigterm handler defined in the main package:

signal.Notify(sigterm, syscall.SIGTERM, syscall.SIGINT, syscall.SIGQUIT)

Making a new signal handler for os.Interrupt does not affect the existing handler which will still terminate the program. I attempted to fix it by calling signal.Reset(os.Interrupt) before adding the new handler to get rid of the existing handler, but I was not able to find a way to re-enable it afterwards. This caused micro to not clean up after itself properly when closed from outside with kill -INT micro.

I think the RunInteractiveShell function would need to have access to the sigterm channel from the main package to be able to re-enable the handler. I have no idea how to accomplish that in a sensible way so I am giving up on trying to fix this issue myself for now.

@niten94
Copy link
Contributor

niten94 commented Jun 9, 2024

I think moving sigterm in the main package to internal/util and readding it in channels receiving SIGINT using signal.Notify in shell.RunInteractiveShell after the command is run can be done.

I do not how is the signal sent but SIGINT is sent to processes in the foreground process group of the terminal where Ctrl+C is pressed. Commands run in micro will be in the same process group so I think the command being run in RunInteractiveShell does not also have to be killed in micro.

I have created a branch in my fork but I realized it may not be good not posting a comment instead first, so I apologize.

dmaluka added a commit to dmaluka/micro that referenced this issue Oct 6, 2024
When we are saving a file with sudo, if we interrupt sudo via Ctrl-c,
it doesn't just kill sudo, it kills micro itself.

The cause is the same as in the issue zyedidia#2612 for RunInteractiveShell()
which was fixed by zyedidia#3357. So fix it the same way as in zyedidia#3357.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants