Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions pipe/command_nil_panic_test.go
Original file line number Diff line number Diff line change
@@ -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.
}
5 changes: 5 additions & 0 deletions pipe/command_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions pipe/command_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading