diff --git a/pipe/command_nil_panic_test.go b/pipe/command_nil_panic_test.go new file mode 100644 index 0000000..740af73 --- /dev/null +++ b/pipe/command_nil_panic_test.go @@ -0,0 +1,47 @@ +//go:build !windows +// +build !windows + +package pipe + +import ( + "context" + "os/exec" + "testing" +) + +func TestKillWithNilProcess(t *testing.T) { + cmd := exec.Command("sleep", "100") + stage := &commandStage{ + name: "test-command", + cmd: cmd, + done: make(chan struct{}), + } + + defer func() { + if r := recover(); r != nil { + t.Fatalf("PANIC OCCURRED (bug not fixed): %v", r) + } + }() + + stage.Kill(context.Canceled) + + t.Log("SUCCESS: Kill() handled nil Process gracefully without panicking") +} + +func TestKillWithFailedStart(t *testing.T) { + ctx := context.Background() + + stage := Command("/this/path/does/not/exist/invalid_command_12345") + + _, err := stage.Start(ctx, Env{}, nil) + if err == nil { + t.Fatal("Expected start to fail, but it succeeded") + } + + // At this point, if someone calls Kill (perhaps from a memory monitor + // or other external component), it could panic if Process is nil + + // Note: In the current implementation, Kill won't be called from the + // context goroutine because Start failed. But external callers like + // MemoryLimit could potentially call Kill on a failed stage. +} diff --git a/pipe/command_unix.go b/pipe/command_unix.go index ce0b3d7..1ccc7a2 100644 --- a/pipe/command_unix.go +++ b/pipe/command_unix.go @@ -30,6 +30,11 @@ func (s *commandStage) Kill(err error) { // use internal synchronization. But those methods only kill the // process, not the process group, so they are not suitable here. + // Check if the process was started successfully before attempting to kill + if s.cmd.Process == nil { + return + } + // We started the process with PGID == PID: pid := s.cmd.Process.Pid select { diff --git a/pipe/command_windows.go b/pipe/command_windows.go index fd285a3..f8cdf3a 100644 --- a/pipe/command_windows.go +++ b/pipe/command_windows.go @@ -9,6 +9,11 @@ func (s *commandStage) runInOwnProcessGroup() {} // kill is called to kill the process if the context expires. `err` is // the corresponding value of `Context.Err()`. func (s *commandStage) Kill(err error) { + // Check if the process was started successfully before attempting to kill + if s.cmd.Process == nil { + return + } + select { case <-s.done: // Process has ended; no need to kill it again.