From 274e71fb6a99c957ab433aeca68071ba74f9bcb0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C5=99emysl=20Eric=20Janouch?= <p@janouch.name>
Date: Mon, 8 Apr 2024 03:22:51 +0200
Subject: [PATCH] Think about process groups

---
 acid.go | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/acid.go b/acid.go
index 007358a..978358f 100644
--- a/acid.go
+++ b/acid.go
@@ -787,6 +787,22 @@ func executorRunTask(ctx context.Context, task Task) error {
 		"ACID_RUNNER="+rt.DB.Runner,
 	)
 
+	// Pushing the runner into a new process group that can be killed at once
+	// with all its children isn't bullet-proof, it messes with job control
+	// when acid is run from an interactive shell, and it also seems avoidable
+	// (use "exec" in runner scripts, so that VMs take over the process).
+	// Maybe this is something that could be opt-in.
+	/*
+		cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+		cmd.Cancel = func() error {
+			err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
+			if err == syscall.ESRCH {
+				return os.ErrProcessDone
+			}
+			return err
+		}
+	*/
+
 	log.Printf("task %d for %s: starting %s\n",
 		rt.DB.ID, rt.DB.FullName(), rt.Runner.Name)
 
@@ -812,10 +828,9 @@ func executorRunTask(ctx context.Context, task Task) error {
 		case <-ctxRunner.Done():
 			// This doesn't leave the runner almost any time on our shutdown,
 			// but whatever--they're supposed to be ephemeral.
-			// Moreover, we don't even override cmd.CancelFunc.
 		case <-time.After(5 * time.Second):
 		}
-		_ = cmd.Process.Kill()
+		_ = cmd.Cancel()
 	}()
 
 	client, err := executorConnect(ctxRunner, &ssh.ClientConfig{