Revert "interp: send interrupt/kill signals to the whole process group" (mvdan#1244)

This reverts commit e256f53 and PR mvdan#1172

This reverted commit introduced a critical trade-off: interactive commands stopped worked (`ssh, `vim`, etc.). Since that fix was for an edge-case, it is not worth keeping that considering its side-effect.

Closes mvdan#1242

Author: andreynering

cmd/gosh/main.go

@@ -11,9 +11,7 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"os/signal"
 	"strings"
-	"syscall"
 
 	"golang.org/x/term"
 
@@ -37,49 +35,48 @@ func main() {
 }
 
 func runAll() error {
-	ctx, _ := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM)
-
 	r, err := interp.New(interp.Interactive(true), interp.StdIO(os.Stdin, os.Stdout, os.Stderr))
 	if err != nil {
 		return err
 	}
 
 	if *command != "" {
-		return run(ctx, r, strings.NewReader(*command), "")
+		return run(r, strings.NewReader(*command), "")
 	}
 	if flag.NArg() == 0 {
 		if term.IsTerminal(int(os.Stdin.Fd())) {
-			return runInteractive(ctx, r, os.Stdin, os.Stdout, os.Stderr)
+			return runInteractive(r, os.Stdin, os.Stdout, os.Stderr)
 		}
-		return run(ctx, r, os.Stdin, "")
+		return run(r, os.Stdin, "")
 	}
 	for _, path := range flag.Args() {
-		if err := runPath(ctx, r, path); err != nil {
+		if err := runPath(r, path); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
-func run(ctx context.Context, r *interp.Runner, reader io.Reader, name string) error {
+func run(r *interp.Runner, reader io.Reader, name string) error {
 	prog, err := syntax.NewParser().Parse(reader, name)
 	if err != nil {
 		return err
 	}
 	r.Reset()
+	ctx := context.Background()
 	return r.Run(ctx, prog)
 }
 
-func runPath(ctx context.Context, r *interp.Runner, path string) error {
+func runPath(r *interp.Runner, path string) error {
 	f, err := os.Open(path)
 	if err != nil {
 		return err
 	}
 	defer f.Close()
-	return run(ctx, r, f, path)
+	return run(r, f, path)
 }
 
-func runInteractive(ctx context.Context, r *interp.Runner, stdin io.Reader, stdout, stderr io.Writer) error {
+func runInteractive(r *interp.Runner, stdin io.Reader, stdout, stderr io.Writer) error {
 	parser := syntax.NewParser()
 	fmt.Fprintf(stdout, "$ ")
 	for stmts, err := range parser.InteractiveSeq(stdin) {
@@ -90,6 +87,7 @@ func runInteractive(ctx context.Context, r *interp.Runner, stdin io.Reader, stdo
 			fmt.Fprintf(stdout, "> ")
 			continue
 		}
+		ctx := context.Background()
 		for _, stmt := range stmts {
 			err := r.Run(ctx, stmt)
 			if r.Exited() {

cmd/gosh/main_test.go

@@ -4,7 +4,6 @@
 package main
 
 import (
-	"context"
 	"fmt"
 	"io"
 	"os"
@@ -203,7 +202,7 @@ func TestInteractive(t *testing.T) {
 			runner, _ := interp.New(interp.Interactive(true), interp.StdIO(inReader, outWriter, outWriter))
 			errc := make(chan error, 1)
 			go func() {
-				errc <- runInteractive(context.Background(), runner, inReader, outWriter, outWriter)
+				errc <- runInteractive(runner, inReader, outWriter, outWriter)
 				// Discard the rest of the input.
 				io.Copy(io.Discard, inReader)
 				inReader.Close()
@@ -256,7 +255,7 @@ func TestInteractiveExit(t *testing.T) {
 	}()
 	w := io.Discard
 	runner, _ := interp.New(interp.Interactive(true), interp.StdIO(inReader, w, w))
-	if err := runInteractive(context.Background(), runner, inReader, w, w); err != nil {
+	if err := runInteractive(runner, inReader, w, w); err != nil {
 		t.Fatal("expected a nil error")
 	}
 }

interp/handler.go

@@ -138,20 +138,19 @@ func DefaultExecHandler(killTimeout time.Duration) ExecHandlerFunc {
 			Stdout: hc.Stdout,
 			Stderr: hc.Stderr,
 		}
-		prepareCommand(&cmd)
 
 		err = cmd.Start()
 		if err == nil {
 			stopf := context.AfterFunc(ctx, func() {
 				if killTimeout <= 0 || runtime.GOOS == "windows" {
-					_ = killCommand(&cmd)
+					_ = cmd.Process.Signal(os.Kill)
 					return
 				}
-				_ = interruptCommand(&cmd)
+				_ = cmd.Process.Signal(os.Interrupt)
 				// TODO: don't sleep in this goroutine if the program
 				// stops itself with the interrupt above.
 				time.Sleep(killTimeout)
-				_ = killCommand(&cmd)
+				_ = cmd.Process.Signal(os.Kill)
 			})
 			defer stopf()
 

interp/os_notunix.go

@@ -8,7 +8,6 @@ package interp
 import (
 	"context"
 	"fmt"
-	"os/exec"
 
 	"mvdan.cc/sh/v3/syntax"
 )
@@ -56,16 +55,3 @@ type waitStatus struct{}
 
 func (waitStatus) Signaled() bool { return false }
 func (waitStatus) Signal() int    { return 0 }
-
-// prepareCommand is a no-op.
-func prepareCommand(cmd *exec.Cmd) {}
-
-// interruptCommand interrupts the process killing it.
-func interruptCommand(cmd *exec.Cmd) error {
-	return cmd.Process.Kill()
-}
-
-// killCommand kills the process by killing it.
-func killCommand(cmd *exec.Cmd) error {
-	return cmd.Process.Kill()
-}

interp/os_unix.go

@@ -7,7 +7,6 @@ package interp
 
 import (
 	"context"
-	"os/exec"
 	"os/user"
 	"strconv"
 	"syscall"
@@ -47,18 +46,3 @@ func (r *Runner) unTestOwnOrGrp(ctx context.Context, op syntax.UnTestOperator, x
 }
 
 type waitStatus = syscall.WaitStatus
-
-// prepareCommand sets the SysProcAttr for the command to create a new process group.
-func prepareCommand(cmd *exec.Cmd) {
-	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
-}
-
-// interruptCommand interrupts the whole process group.
-func interruptCommand(cmd *exec.Cmd) error {
-	return unix.Kill(-cmd.Process.Pid, unix.SIGINT)
-}
-
-// killCommand kills the whole process group.
-func killCommand(cmd *exec.Cmd) error {
-	return unix.Kill(-cmd.Process.Pid, unix.SIGKILL)
-}