diff --git a/cmd/run.go b/cmd/run.go index 5ffecd591..f7a1e3182 100644 --- 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 index 35c61ae84..6dcd68358 100644 --- 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 new file mode 100644 index 000000000..2f5a48dcf --- /dev/null +++ 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 index eb95a1d4b..77cbc6408 100644 --- 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()