Ensure the child process receives SIGTERM when parent dies on Linux

Makefile

@@ -39,7 +39,7 @@ build: $(current_binary) ## Build the func-e binary
 test_packages := . ./internal/...
 test: ## Run all unit tests
 	@printf "$(ansi_format_dark)" test "running unit tests"
-	@go test . $(test_packages)
+	@go test -timeout 30s . $(test_packages)
 	@printf "$(ansi_format_bright)" test "ok"
 
 coverage:  ## Generate test coverage

internal/moreos/moreos_test.go

@@ -15,6 +15,7 @@
 package moreos
 
 import (
+	"bufio"
 	"bytes"
 	"errors"
 	"fmt"
@@ -23,6 +24,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"testing"
+	"time"
 
 	"github.com/shirou/gopsutil/v3/process"
 	"github.com/stretchr/testify/require"
@@ -120,7 +122,7 @@ func TestSprintf_IdiomaticPerOS(t *testing.T) {
 
 func TestProcessGroupAttr_Interrupt(t *testing.T) {
 	// Fork a process that hangs
-	cmd := exec.Command("cat" + Exe)
+	cmd := exec.Command("sleep"+Exe, "1000")
 	cmd.SysProcAttr = ProcessGroupAttr()
 	require.NoError(t, cmd.Start())
 
@@ -140,7 +142,7 @@ func TestProcessGroupAttr_Interrupt(t *testing.T) {
 
 func Test_EnsureProcessDone(t *testing.T) {
 	// Fork a process that hangs
-	cmd := exec.Command("cat" + Exe)
+	cmd := exec.Command("sleep"+Exe, "1000")
 	cmd.SysProcAttr = ProcessGroupAttr()
 	require.NoError(t, cmd.Start())
 
@@ -155,6 +157,65 @@ func Test_EnsureProcessDone(t *testing.T) {
 	require.NoError(t, EnsureProcessDone(cmd.Process))
 }
 
+func Test_EnsureChildProcessDone(t *testing.T) {
+	// There are 3 processes spawned by this test target, with the following relationship:
+	//
+	// runner (go run testdata/main/main.go)
+	// └── main (the executable, compiled from testdata/main/main.go. This is a fake func-e)
+	//     └── child (sleep, this is a fake envoy)
+	cmd := exec.Command("go"+Exe, "run", filepath.Join("testdata", "main", "main.go"))
+	stdout, err := cmd.StdoutPipe()
+	require.NoError(t, err)
+	cmd.Start()
+
+	buf := bufio.NewReader(stdout)
+	for {
+		line, _, readErr := buf.ReadLine()
+		require.NoError(t, readErr)
+		// As soon as we detect output from the main program, we know the child process in main program
+		// is started. See: testdata/main/main.go#34.
+		if string(line) == "ok" {
+			break
+		}
+	}
+
+	// This is the "go run" process.
+	runner, err := process.NewProcess(int32(cmd.Process.Pid))
+	require.NoError(t, err)
+
+	parents, err := runner.Children()
+	require.NoError(t, err)
+	require.Equal(t, len(parents), 1)
+
+	// This is the main process executed by "go run".
+	main, err := process.NewProcess(parents[0].Pid)
+	require.NoError(t, err)
+
+	children, err := main.Children()
+	require.NoError(t, err)
+	require.Equal(t, len(children), 1)
+
+	// Kill the main process.
+	require.NoError(t, EnsureProcessDone(&os.Process{Pid: int(main.Pid)}))
+
+	// Wait and check the status of child and main processes.
+	// The following check is here to make sure the child and main processes are not killed because
+	// of the runner is died.
+	time.Sleep(time.Millisecond)
+	child := children[0]
+	require.Error(t, findProcess(&os.Process{Pid: int(child.Pid)}))
+	require.Error(t, findProcess(&os.Process{Pid: int(main.Pid)}))
+
+	// The runner needs to be killed too.
+	require.NoError(t, EnsureProcessDone(&os.Process{Pid: int(runner.Pid)}))
+	cmd.Wait() //nolint
+
+	require.Error(t, findProcess(&os.Process{Pid: int(runner.Pid)}))
+
+	// Ensure killing it again doesn't error.
+	require.NoError(t, EnsureProcessDone(&os.Process{Pid: int(main.Pid)}))
+}
+
 func findProcess(proc *os.Process) error {
 	_, err := process.NewProcess(int32(proc.Pid)) // because os.FindProcess is no-op in Linux!
 	return err

internal/moreos/proc_darwin.go

@@ -17,6 +17,8 @@ package moreos
 import (
 	"os"
 	"syscall"
+
+	"github.com/shirou/gopsutil/v3/process"
 )
 
 const exe = ""
@@ -33,7 +35,24 @@ func interrupt(p *os.Process) error {
 }
 
 func ensureProcessDone(p *os.Process) error {
-	if err := p.Kill(); err != nil && err != os.ErrProcessDone {
+	proc, err := process.NewProcess(int32(p.Pid))
+	if err != nil {
+		if err == process.ErrorProcessNotRunning {
+			return nil
+		}
+		return err
+	}
+
+	// on macOS, when a process doesn't have children, pgrep gives "exit status 1", hence we ignore
+	// the error here.
+	children, _ := proc.Children()
+	for _, child := range children {
+		if err := child.Kill(); err != nil && err != process.ErrorProcessNotRunning {
+			return err
+		}
+	}
+
+	if err := proc.Kill(); err != nil && err != os.ErrProcessDone {
 		return err
 	}
 	return nil

internal/moreos/testdata/main/main.go

@@ -0,0 +1,37 @@
+// Copyright 2021 Tetrate
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"os/exec"
+	"os/signal"
+	"syscall"
+
+	"github.com/tetratelabs/func-e/internal/moreos"
+)
+
+func main() {
+	ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGKILL)
+	defer stop()
+	cmd := exec.Command("sleep"+moreos.Exe, "1000")
+	cmd.SysProcAttr = moreos.ProcessGroupAttr()
+	cmd.Start()
+	fmt.Fprintln(os.Stdout, "ok")
+	cmd.Wait()
+	<-ctx.Done()
+}