Skip to content

Commit 17b8fad

Browse files
committed
Address copilot comments
Signed-off-by: Albert Callarisa <albert@diagrid.io>
1 parent 2788629 commit 17b8fad

File tree

2 files changed

+30
-8
lines changed

2 files changed

+30
-8
lines changed

cmd/run_unix.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,30 +16,51 @@ limitations under the License.
1616
package cmd
1717

1818
import (
19+
"errors"
1920
"fmt"
2021
"os"
2122
"os/exec"
2223
"syscall"
24+
"time"
2325

2426
"github.com/dapr/cli/pkg/print"
2527
runExec "github.com/dapr/cli/pkg/runexec"
2628
)
2729

2830
// killProcessGroup kills the entire process group of the given process so that
2931
// grandchild processes (e.g. the compiled binary spawned by `go run`) are also
30-
// terminated. It sends SIGTERM first and falls back to SIGKILL on failure.
32+
// terminated. It sends SIGTERM first; if the process group is still alive after
33+
// a 5-second grace period, it sends SIGKILL.
3134
func killProcessGroup(process *os.Process) error {
3235
pgid, err := syscall.Getpgid(process.Pid)
3336
if err != nil {
34-
return process.Kill() // fallback: can't determine pgid
35-
}
36-
err = syscall.Kill(-pgid, syscall.SIGTERM)
37-
if err != nil && err != syscall.ESRCH {
38-
err = syscall.Kill(-pgid, syscall.SIGKILL)
37+
if err == syscall.ESRCH {
38+
return nil // process already gone
39+
}
40+
// Can't determine pgid for some other reason — fall back to single-process kill.
41+
if killErr := process.Kill(); errors.Is(killErr, os.ErrProcessDone) {
42+
return nil
43+
} else {
44+
return killErr
45+
}
3946
}
40-
if err == syscall.ESRCH {
47+
48+
if err := syscall.Kill(-pgid, syscall.SIGTERM); err == syscall.ESRCH {
4149
return nil // process group already gone
4250
}
51+
52+
const gracePeriod = 5 * time.Second
53+
deadline := time.Now().Add(gracePeriod)
54+
for time.Now().Before(deadline) {
55+
if err := syscall.Kill(-pgid, 0); err == syscall.ESRCH {
56+
return nil // process group gone
57+
}
58+
time.Sleep(100 * time.Millisecond)
59+
}
60+
// Grace period elapsed — force kill.
61+
if err := syscall.Kill(-pgid, syscall.SIGKILL); err == syscall.ESRCH {
62+
return nil
63+
}
4364
return err
4465
}
4566

cmd/run_windows.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ import (
2424
runExec "github.com/dapr/cli/pkg/runexec"
2525
)
2626

27-
// killProcessGroup on Windows delegates to Process.Kill; job objects already handle child cleanup.
27+
// killProcessGroup on Windows delegates to Process.Kill; this helper does not manage
28+
// child processes via job objects in the single-app run path.
2829
func killProcessGroup(process *os.Process) error {
2930
return process.Kill()
3031
}

0 commit comments

Comments
 (0)