Terminate app process group on exit (#1602)

acroca · github-actions[bot] · commit 432ef1e31d6f · 2026-03-06T05:54:42.000Z
* Kill app process group when stopping Signed-off-by: Albert Callarisa <albert@diagrid.io> * Address copilot comments Signed-off-by: Albert Callarisa <albert@diagrid.io> * Addressed copilot comments Signed-off-by: Albert Callarisa <albert@diagrid.io> * Addressed copilot comments Signed-off-by: Albert Callarisa <albert@diagrid.io> * Addressed copilot comments Signed-off-by: Albert Callarisa <albert@diagrid.io> * Addressed copilot comments Signed-off-by: Albert Callarisa <albert@diagrid.io> * Addressed copilot comments Signed-off-by: Albert Callarisa <albert@diagrid.io> --------- Signed-off-by: Albert Callarisa <albert@diagrid.io> (cherry picked from commit 35cba49)
diff --git a/cmd/run.go b/cmd/run.go
@@ -443,8 +443,8 @@ dapr run --run-file /path/to/directory -k
 		if output.AppErr != nil {
 			exitWithError = true
 			print.FailureStatusEvent(os.Stderr, fmt.Sprintf("Error exiting App: %s", output.AppErr))
-		} else if output.AppCMD != nil && output.AppCMD.Process != nil && (output.AppCMD.ProcessState == nil || !output.AppCMD.ProcessState.Exited()) {
-			err = output.AppCMD.Process.Kill()
+		} else if output.AppCMD != nil && output.AppCMD.Process != nil {
+			err = killProcessGroup(output.AppCMD.Process)
 			if err != nil {
 				// If the process already exited on its own, treat this as a clean shutdown.
 				if errors.Is(err, os.ErrProcessDone) {
@@ -826,7 +826,7 @@ func stopDaprdAndAppProcesses(runState *runExec.RunExec) bool {
 	if appErr != nil {
 		exitWithError = true
 		print.StatusEvent(runState.AppCMD.ErrorWriter, print.LogFailure, "Error exiting App: %s", appErr)
-	} else if runState.AppCMD.Command != nil && runState.AppCMD.Command.Process != nil && (runState.AppCMD.Command.ProcessState == nil || !runState.AppCMD.Command.ProcessState.Exited()) {
+	} else if runState.AppCMD.Command != nil && runState.AppCMD.Command.Process != nil {
 		err = killAppProcess(runState)
 		if err != nil {
 			exitWithError = true
@@ -1002,12 +1002,7 @@ func killAppProcess(runE *runExec.RunExec) error {
 	if runE.AppCMD.Command == nil || runE.AppCMD.Command.Process == nil {
 		return nil
 	}
-	// Check if the process has already exited on its own.
-	if runE.AppCMD.Command.ProcessState != nil && runE.AppCMD.Command.ProcessState.Exited() {
-		// Process already exited, no need to kill it.
-		return nil
-	}
-	err := runE.AppCMD.Command.Process.Kill()
+	err := killProcessGroup(runE.AppCMD.Command.Process)
 	if err != nil {
 		// If the process already exited on its own
 		if errors.Is(err, os.ErrProcessDone) {
diff --git a/cmd/run_unix.go b/cmd/run_unix.go
@@ -16,15 +16,77 @@ limitations under the License.
 package cmd
 
 import (
+	"errors"
 	"fmt"
 	"os"
 	"os/exec"
 	"syscall"
+	"time"
 
 	"github.com/dapr/cli/pkg/print"
 	runExec "github.com/dapr/cli/pkg/runexec"
 )
 
+// killProcessGroup kills the entire process group of the given process so that
+// grandchild processes (e.g. the compiled binary spawned by `go run`) are also
+// terminated. It sends SIGTERM first; if the process group is still alive after
+// a 5-second grace period, it sends SIGKILL.
+func killProcessGroup(process *os.Process) error {
+	var (
+		pgid int
+		err  error
+	)
+
+	pgid, err = syscall.Getpgid(process.Pid)
+	if err != nil {
+		if errors.Is(err, syscall.ESRCH) {
+			// The group leader may have already exited (e.g. when using `go run`),
+			// but other processes in the same process group can still be alive.
+			// Since the app is started with Setpgid=true, the PGID equals the leader
+			// PID, so fall back to using process.Pid as the PGID.
+			pgid = process.Pid
+		} else {
+			// Can't determine pgid for some other reason — fall back to single-process kill.
+			killErr := process.Kill()
+			if errors.Is(killErr, os.ErrProcessDone) {
+				return nil
+			}
+			return killErr
+		}
+	}
+
+	err = syscall.Kill(-pgid, syscall.SIGTERM)
+	if err != nil {
+		if err == syscall.ESRCH {
+			return nil // process group already gone
+		}
+		return fmt.Errorf("failed to send SIGTERM to process group %d: %w", pgid, err)
+	}
+
+	const gracePeriod = 5 * time.Second
+	deadline := time.Now().Add(gracePeriod)
+	for time.Now().Before(deadline) {
+		err = syscall.Kill(-pgid, 0)
+		if err == nil {
+			time.Sleep(100 * time.Millisecond)
+			continue
+		}
+		if errors.Is(err, syscall.ESRCH) {
+			return nil // process group gone
+		}
+		return fmt.Errorf("failed to check status of process group %d: %w", pgid, err)
+	}
+	// Grace period elapsed — force kill.
+	err = syscall.Kill(-pgid, syscall.SIGKILL)
+	if err == syscall.ESRCH {
+		return nil
+	}
+	if err != nil {
+		return fmt.Errorf("failed to send SIGKILL to process group %d: %w", pgid, err)
+	}
+	return nil
+}
+
 // setDaprProcessGroupForRun sets the process group on the daprd command so the
 // sidecar can be managed independently (e.g. when the app is started via exec).
 func setDaprProcessGroupForRun(cmd *exec.Cmd) {
diff --git a/cmd/run_unix_test.go b/cmd/run_unix_test.go
@@ -0,0 +1,97 @@
+//go:build !windows
+
+/*
+Copyright 2026 The Dapr Authors
+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 cmd
+
+import (
+	"os/exec"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestKillProcessGroup_KillsEntireGroup verifies that killProcessGroup terminates all
+// processes in the group, not just the leader. A second process is explicitly placed
+// into the leader's process group using SysProcAttr.Pgid; both must be gone after
+// killProcessGroup returns.
+func TestKillProcessGroup_KillsEntireGroup(t *testing.T) {
+	leader := exec.Command("sleep", "100")
+	leader.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	require.NoError(t, leader.Start())
+
+	// Explicitly place a second process in the same process group as the leader.
+	member := exec.Command("sleep", "100")
+	member.SysProcAttr = &syscall.SysProcAttr{Setpgid: true, Pgid: leader.Process.Pid}
+	require.NoError(t, member.Start())
+
+	// Wait goroutines reap the processes once killed, so Kill(-pgid, 0) eventually
+	// returns ESRCH rather than seeing zombies that still count as group members.
+	go func() { _ = leader.Wait() }()
+	go func() { _ = member.Wait() }()
+
+	require.NoError(t, killProcessGroup(leader.Process))
+
+	assert.Eventually(t, func() bool {
+		return syscall.Kill(-leader.Process.Pid, 0) == syscall.ESRCH
+	}, 2*time.Second, 50*time.Millisecond, "process group should be gone after killProcessGroup")
+}
+
+// TestKillProcessGroup_AlreadyGone verifies that killProcessGroup returns nil when
+// the process has already exited cleanly before kill is attempted.
+func TestKillProcessGroup_AlreadyGone(t *testing.T) {
+	cmd := exec.Command("true")
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	require.NoError(t, cmd.Start())
+	proc := cmd.Process
+	require.NoError(t, cmd.Wait())
+
+	assert.NoError(t, killProcessGroup(proc))
+}
+
+// TestKillProcessGroup_LeaderExitedGroupAlive verifies that killProcessGroup still
+// kills remaining group members when the leader has already exited — the typical
+// `go run` scenario where the wrapper process exits but the compiled binary lives on
+// in the same process group.
+func TestKillProcessGroup_LeaderExitedGroupAlive(t *testing.T) {
+	leader := exec.Command("sleep", "100")
+	leader.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	require.NoError(t, leader.Start())
+	leaderPGID := leader.Process.Pid
+
+	// Place a second process in the same group as the leader.
+	member := exec.Command("sleep", "100")
+	member.SysProcAttr = &syscall.SysProcAttr{Setpgid: true, Pgid: leaderPGID}
+	require.NoError(t, member.Start())
+
+	// Kill only the group leader; member stays alive in the same group.
+	require.NoError(t, leader.Process.Kill())
+	_ = leader.Wait() // reap immediately so it doesn't linger as a zombie
+
+	// Confirm the group still has a live member before we call killProcessGroup.
+	require.NoError(t, syscall.Kill(-leaderPGID, 0), "member should still be alive in the group")
+
+	// killProcessGroup falls back to process.Pid as PGID since Getpgid returns ESRCH
+	// for the dead leader, then signals and terminates the remaining member.
+	go func() { _ = member.Wait() }()
+
+	require.NoError(t, killProcessGroup(leader.Process))
+
+	assert.Eventually(t, func() bool {
+		return syscall.Kill(-leaderPGID, 0) == syscall.ESRCH
+	}, 2*time.Second, 50*time.Millisecond, "process group should be gone after leader exits")
+}
diff --git a/cmd/run_windows.go b/cmd/run_windows.go
@@ -19,11 +19,33 @@ import (
 	"fmt"
 	"os"
 	"os/exec"
+	"strconv"
+
+	"github.com/kolesnikovae/go-winjob"
 
 	"github.com/dapr/cli/pkg/print"
 	runExec "github.com/dapr/cli/pkg/runexec"
+	"github.com/dapr/cli/utils"
 )
 
+// killProcessGroup on Windows terminates the entire process tree by terminating the
+// job object associated with the current CLI process (keyed by os.Getpid()). The
+// process argument is only used for the fallback path when no job object can be
+// opened (e.g. the process was never attached to one).
+func killProcessGroup(process *os.Process) error {
+	jobName := utils.GetJobObjectNameFromPID(strconv.Itoa(os.Getpid()))
+	jbobj, err := winjob.Open(jobName)
+	if err != nil {
+		if os.IsNotExist(err) {
+			// Job object not found — process was never attached. Fall back to single-process kill.
+			return process.Kill()
+		}
+		return fmt.Errorf("failed to open job object %q: %w", jobName, err)
+	}
+	defer jbobj.Close()
+	return jbobj.TerminateWithExitCode(0)
+}
+
 // setDaprProcessGroupForRun is a no-op on Windows (SysProcAttr.Setpgid does not exist).
 func setDaprProcessGroupForRun(cmd *exec.Cmd) {
 	// no-op on Windows
@@ -48,6 +70,7 @@ func startAppProcessInBackground(output *runExec.RunOutput, binary string, args
 	if err := output.AppCMD.Start(); err != nil {
 		return fmt.Errorf("failed to start app: %w", err)
 	}
+	utils.AttachJobObjectToProcess(strconv.Itoa(os.Getpid()), output.AppCMD.Process)
 
 	go func() {
 		waitErr := output.AppCMD.Wait()
