v2 bug: control-c does not kill process

[bonesyo]

Checklist

• Are you running the latest v2 release? The list of releases is here.
• Did you check the manual for your release? The v2 manual is here
• Did you perform a search about this problem? Here's the Github guide about searching.

Describe the bug

When a creating even a simple long running process, you can no longer use control-c to kill the process.

To reproduce

package main

import (
	"log"
	"os"
	"time"

	"github.com/urfave/cli"
)

func main() {
	app := &cli.App{
		Name:  "boom",
		Usage: "make an explosive entrance",
		Action: func(c *cli.Context) error {
			for {
				time.Sleep(time.Second)
			}
			return nil
		},
	}

	err := app.Run(os.Args)
	if err != nil {
		log.Fatal(err)
	}
}

Expected behavior

Process to terminate when hitting control-c

Additional context

control-c works when using v1 release, but not v2

Want to fix this yourself?

We'd love to have more contributors on this project! If the fix for this bug is easily explained and very small, free free to create a pull request for it.

Run go env and paste its output here

GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/jamesontiveros/Library/Caches/go-build"
GOENV="/Users/jamesontiveros/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GONOPROXY="github.com/use-weave*"
GONOSUMDB="github.com/use-weave*"
GOOS="darwin"
GOPATH="/Users/jamesontiveros/go"
GOPRIVATE="github.com/use-weave*"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.13.4/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.13.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/jamesontiveros/go/src/github.com/urfave/cli/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/pd/f_28x1bn43vft8sj70l9t5j40000gn/T/go-build216677680=/tmp/go-build -gno-record-gcc-switches -fno-common"

[AudriusButkevicius]

This is a command line argument parsing library, not a process lifetime management library, and the user of the library should handle this themselves.

Essentially, I think it's the right decision not to handle this for the user, as this takes away the ability for the user to handle graceful termination.

[tpihl]

While i agree that process lifetime is outside this libs responsibility, I cannot find any way to pass a parent context into this lib (that can be cancelled as part of the lifetime).

What is the correct way to implement a context-based terminate signal?

[AudriusButkevicius]

Yeah, it seems the API is lack-luster. Perhaps there should be RunWithContext.

[bonesyo]

This is what I ended up adding:

sigs := make(chan os.Signal, 1)
signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM)

go func() {
	<-sigs
	os.Exit(1)
}()

[tpihl]

@bonesyo I really really don't want to go that way since i depend on my defer's in all running go-routines to actually shut things down correctly.

I admit to put a panic on a timer to be sure there is an actual termination if something go wrong, but os.Exit will never close or flush or sync anything, and I need SIGTERM to be a normal way to terminate when a subcommand is a server-thing.

@AudriusButkevicius that would solve things for me.

[Sytten]

FYI there is a RunWithContext now

[tpihl]

Super!!

Ref #945