mage icon indicating copy to clipboard operation
mage copied to clipboard

Signal can't be caught by the underlying binary

Open alon7 opened this issue 5 years ago • 6 comments

Hi everyone

Moved my code from makefile to mage and I'm very pleased so thank you for the great work!

I'm running another go binary which has a loop running and this code to catch signals

stopChan := make(chan os.Signal)
	signal.Notify(stopChan, syscall.SIGTERM, syscall.SIGINT, os.Interrupt, syscall.SIGQUIT, syscall.SIGHUP)

then this code a bit later:

<-stopChan
log.Info("Shutting down gracefully...")

but when i ctrl + c my mage running process the underlying process doesn't receive any signal. I've seen it with other executables that do cleanup on terminal as well such as terraform.

Let me know if you have any idea on how to approach a fix here or maybe if I'm missing something.

Thanks!

alon7 avatar Oct 20 '19 20:10 alon7

Yeah, thanks for bringing this up, that's something we should make work correctly - passing a signal down from mage to the commands you're executing. It will take some work, but I think it's doable.

natefinch avatar Oct 21 '19 19:10 natefinch

Thanks for the quick response. If you can direct me a bit I might be able to fix it over a weekend

alon7 avatar Oct 21 '19 20:10 alon7

Just wanted to check if anyone is working on this before opening a PR.

pmcatominey avatar Jul 31 '20 22:07 pmcatominey

I don't believe the linked PR addresses this issue. If I'm reading this right, it will issue a SigKill to the underlying process which doesn't seem like what this issue is talking about. For example, let's say I wanted a mage command that spins up a service with docker compose and lets it run until I'm done with it. If I issue a sigint to spin it down, it looks like #313 would send it a kill. To actually pass through the sigint, I believe you'd have to do something like the following (lifted from here):

if err := cmd.Start(); err != nil {
    log.Fatal(err) // Command not found on PATH, not executable, &c.
}
// wait for the command to finish
waitCh := make(chan error, 1)
go func() {
    waitCh <- cmd.Wait()
    close(waitCh)
}()
sigChan := make(chan os.Signal, 1)
signal.Notify(sigChan)

// You need a for loop to handle multiple signals
for {
    select {
    case sig := <-sigChan:
        if err := cmd.Process.Signal(sig); err != nil {
            // Not clear how we can hit this, but probably not
            // worth terminating the child.
            log.Print("error sending signal", sig, err)
        }
    case err := <-waitCh:
        // Subprocess exited. Get the return code, if we can
        var waitStatus syscall.WaitStatus
        if exitError, ok := err.(*exec.ExitError); ok {
            waitStatus = exitError.Sys().(syscall.WaitStatus)
            os.Exit(waitStatus.ExitStatus())
        }
        if err != nil {
            log.Fatal(err)
        }
        return
    }
}

flowchartsman avatar Oct 23 '20 00:10 flowchartsman

Hi @flowchartsman,

My PR will allow the magefile to catch a sigint, this is covered with this test, sigkill is issued after a second sigint is received.

pmcatominey avatar Oct 24 '20 10:10 pmcatominey

@pmcatominey Thanks for working on this. I am new to Mage and was wondering if this could help with having mage start running 2(or more) depended targets at once(run with mg.Deps), and canceling(terminate) them when issuing Ctrl-C on mage process?

My use case is having a Go project consisting of multiple micro-services in cmd/ and want to start the whole thing at once with Mage. Thanks!

Dragomir-Ivanov avatar Oct 31 '20 20:10 Dragomir-Ivanov