Fix signal handling for subprocess

Taskfile.yml

@@ -45,16 +45,23 @@ tasks:
     desc: Cleans temp files and folders
     cmds:
       - rm -rf dist/
+      - rm -rf tmp/
 
   lint:
     desc: Runs golint
     cmds:
       - golint {{catLines .GO_PACKAGES}}
     silent: true
 
+  sleepit:
+    desc: Builds the sleepit test helper
+    dir: tmp
+    cmds:
+      - go build ../internal/sleepit
+
   test:
     desc: Runs test suite
-    deps: [install]
+    deps: [install, sleepit]
     cmds:
       - go test {{catLines .GO_PACKAGES}}
 

internal/execext/exec.go

@@ -55,7 +55,16 @@ func RunCommand(ctx context.Context, opts *RunCommandOptions) error {
 	if err != nil {
 		return err
 	}
-	return r.Run(ctx, p)
+
+	// We used to pass to interp.Runner a context that was cancelled on reception of a
+	// OS signal. This caused the Runner to terminate the subprocess abruptly.
+	// The correct behavior instead is for us to completely ignore the signal and let
+	// the subprocess deal with it. If the subprocess doesn't handle the signal, it will
+	// be terminated. If the subprocess does handle the signal, it knows better than us
+	// wether it wants to cleanup and terminate or do something different.
+	// See https://github.com/go-task/task/issues/458 for details.
+	// So now we pass an empty context just to make the API of interp.Runner happy
+	return r.Run(context.Background(), p)
 }
 
 // IsExitError returns true the given error is an exis status error

internal/sleepit/main.go

@@ -0,0 +1,176 @@
+// This code is released under the MIT License
+// Copyright (c) 2020 Marco Molteni and the timeit contributors.
+
+package main
+
+import (
+	"flag"
+	"fmt"
+	"os"
+	"os/signal"
+	"time"
+)
+
+const usage = `sleepit: sleep for the specified duration, optionally handling signals
+When the line "sleepit: ready" is printed, it means that it is safe to send signals to it
+
+Usage: sleepit <command> [<args>]
+
+Commands
+
+  default     Use default action: on reception of SIGINT terminate abruptly
+  handle      Handle signals: on reception of SIGINT perform cleanup before exiting
+  version     Show the sleepit version`
+
+var (
+	// Filled by the linker.
+	fullVersion = "unknown" // example: v0.0.9-8-g941583d027-dirty
+)
+
+func main() {
+	os.Exit(run(os.Args[1:]))
+}
+
+func run(args []string) int {
+	if len(args) < 1 {
+		fmt.Fprintln(os.Stderr, usage)
+		return 2
+	}
+
+	defaultCmd := flag.NewFlagSet("default", flag.ExitOnError)
+	defaultSleep := defaultCmd.Duration("sleep", 5*time.Second, "Sleep duration")
+
+	handleCmd := flag.NewFlagSet("handle", flag.ExitOnError)
+	handleSleep := handleCmd.Duration("sleep", 5*time.Second, "Sleep duration")
+	handleCleanup := handleCmd.Duration("cleanup", 5*time.Second, "Cleanup duration")
+	handleTermAfter := handleCmd.Int("term-after", 0,
+		"Terminate immediately after `N` signals.\n"+
+			"Default is to terminate only when the cleanup phase has completed.")
+
+	versionCmd := flag.NewFlagSet("version", flag.ExitOnError)
+
+	switch args[0] {
+
+	case "default":
+		defaultCmd.Parse(args[1:])
+		if len(defaultCmd.Args()) > 0 {
+			fmt.Fprintf(os.Stderr, "default: unexpected arguments: %v\n", defaultCmd.Args())
+			return 2
+		}
+		return supervisor(*defaultSleep, 0, 0, nil)
+
+	case "handle":
+		handleCmd.Parse(args[1:])
+		if *handleTermAfter == 1 {
+			fmt.Fprintf(os.Stderr, "handle: term-after cannot be 1\n")
+			return 2
+		}
+		if len(handleCmd.Args()) > 0 {
+			fmt.Fprintf(os.Stderr, "handle: unexpected arguments: %v\n", handleCmd.Args())
+			return 2
+		}
+		sigCh := make(chan os.Signal, 1)
+		signal.Notify(sigCh, os.Interrupt) // Ctrl-C -> SIGINT
+		return supervisor(*handleSleep, *handleCleanup, *handleTermAfter, sigCh)
+
+	case "version":
+		versionCmd.Parse(args[1:])
+		if len(versionCmd.Args()) > 0 {
+			fmt.Fprintf(os.Stderr, "version: unexpected arguments: %v\n", versionCmd.Args())
+			return 2
+		}
+		fmt.Printf("sleepit version %s\n", fullVersion)
+		return 0
+
+	default:
+		fmt.Fprintln(os.Stderr, usage)
+		return 2
+	}
+}
+
+func supervisor(
+	sleep time.Duration,
+	cleanup time.Duration,
+	termAfter int,
+	sigCh <-chan os.Signal,
+) int {
+	fmt.Printf("sleepit: ready\n")
+	fmt.Printf("sleepit: PID=%d sleep=%v cleanup=%v\n",
+		os.Getpid(), sleep, cleanup)
+
+	cancelWork := make(chan struct{})
+	workerDone := worker(cancelWork, sleep, "work")
+
+	cancelCleaner := make(chan struct{})
+	var cleanerDone <-chan struct{}
+
+	sigCount := 0
+	for {
+		select {
+		case sig := <-sigCh:
+			sigCount++
+			fmt.Printf("sleepit: got signal=%s count=%d\n", sig, sigCount)
+			if sigCount == 1 {
+				// since `cancelWork` is unbuffered, sending will be synchronous:
+				// we are ensured that the worker has terminated before starting cleanup.
+				// This is important in some real-life situations.
+				cancelWork <- struct{}{}
+				cleanerDone = worker(cancelCleaner, cleanup, "cleanup")
+			}
+			if sigCount == termAfter {
+				cancelCleaner <- struct{}{}
+				return 4
+			}
+		case <-workerDone:
+			return 0
+		case <-cleanerDone:
+			return 3
+		}
+	}
+}
+
+// Start a worker goroutine and return immediately a `workerDone` channel.
+// The goroutine will prepend its prints with the prefix `name`.
+// The goroutine will simulate some work and will terminate when one of the following
+// conditions happens:
+// 1. When `howlong` is elapsed. This case will be signaled on the `workerDone` channel.
+// 2. When something happens on channel `canceled`. Note that this simulates real-life,
+//    so cancellation is not instantaneous: if the caller wants a synchronous cancel,
+//    it should send a message; if instead it wants an asynchronous cancel, it should
+//    close the channel.
+func worker(
+	canceled <-chan struct{},
+	howlong time.Duration,
+	name string,
+) <-chan struct{} {
+	workerDone := make(chan struct{})
+	deadline := time.Now().Add(howlong)
+	go func() {
+		fmt.Printf("sleepit: %s started\n", name)
+		for {
+			select {
+			case <-canceled:
+				fmt.Printf("sleepit: %s canceled\n", name)
+				return
+			default:
+				if doSomeWork(deadline) {
+					fmt.Printf("sleepit: %s done\n", name) // <== NOTE THIS LINE
+					workerDone <- struct{}{}
+					return
+				}
+			}
+		}
+	}()
+	return workerDone
+}
+
+// Do some work and then return, so that the caller can decide wether to continue or not.
+// Return true when all work is done.
+func doSomeWork(deadline time.Time) bool {
+	if time.Now().After(deadline) {
+		return true
+	}
+	timeout := 100 * time.Millisecond
+	time.Sleep(timeout)
+	return false
+}

testdata/ignore_signals/Taskfile.yml

@@ -0,0 +1,6 @@
+version: '3'
+
+tasks:
+  default:
+    cmds:
+      - '{{.CLI_ARGS}}'