Preserve subprocess wait failures
This commit is contained in:
@@ -2,6 +2,7 @@ package subprocess
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
"time"
|
"time"
|
||||||
)
|
)
|
||||||
@@ -58,10 +59,10 @@ func executeOwnedCommand(ctx context.Context, cmd *exec.Cmd, abort <-chan error)
|
|||||||
} else {
|
} else {
|
||||||
outcome.reason = executionCanceled
|
outcome.reason = executionCanceled
|
||||||
}
|
}
|
||||||
outcome.cleanupErr = terminateAndReap(tree, waitCh)
|
outcome.waitErr, outcome.cleanupErr = terminateAndReap(tree, waitCh)
|
||||||
case outcome.abortErr = <-abort:
|
case outcome.abortErr = <-abort:
|
||||||
outcome.reason = executionAborted
|
outcome.reason = executionAborted
|
||||||
outcome.cleanupErr = terminateAndReap(tree, waitCh)
|
outcome.waitErr, outcome.cleanupErr = terminateAndReap(tree, waitCh)
|
||||||
}
|
}
|
||||||
if cmd.ProcessState != nil {
|
if cmd.ProcessState != nil {
|
||||||
outcome.exitCode = cmd.ProcessState.ExitCode()
|
outcome.exitCode = cmd.ProcessState.ExitCode()
|
||||||
@@ -69,22 +70,22 @@ func executeOwnedCommand(ctx context.Context, cmd *exec.Cmd, abort <-chan error)
|
|||||||
return outcome
|
return outcome
|
||||||
}
|
}
|
||||||
|
|
||||||
func terminateAndReap(tree ownedProcessTree, waitCh <-chan error) error {
|
func terminateAndReap(tree ownedProcessTree, waitCh <-chan error) (waitErr, cleanupErr error) {
|
||||||
cleanupErr := tree.TerminateGracefully()
|
cleanupErr = tree.TerminateGracefully()
|
||||||
timer := time.NewTimer(gracefulTerminationWait)
|
timer := time.NewTimer(gracefulTerminationWait)
|
||||||
defer timer.Stop()
|
defer timer.Stop()
|
||||||
select {
|
select {
|
||||||
case <-waitCh:
|
case waitErr = <-waitCh:
|
||||||
return joinErrors(cleanupErr, tree.TerminateForcefully())
|
return waitErr, joinErrors(cleanupErr, tree.TerminateForcefully())
|
||||||
case <-timer.C:
|
case <-timer.C:
|
||||||
}
|
}
|
||||||
cleanupErr = joinErrors(cleanupErr, tree.TerminateForcefully())
|
cleanupErr = joinErrors(cleanupErr, tree.TerminateForcefully())
|
||||||
forcefulTimer := time.NewTimer(forcefulTerminationWait)
|
forcefulTimer := time.NewTimer(forcefulTerminationWait)
|
||||||
defer forcefulTimer.Stop()
|
defer forcefulTimer.Stop()
|
||||||
select {
|
select {
|
||||||
case <-waitCh:
|
case waitErr = <-waitCh:
|
||||||
return cleanupErr
|
return waitErr, cleanupErr
|
||||||
case <-forcefulTimer.C:
|
case <-forcefulTimer.C:
|
||||||
return joinErrors(cleanupErr, context.DeadlineExceeded)
|
return nil, joinErrors(cleanupErr, fmt.Errorf("owned subprocess did not reap within %s after forceful termination", forcefulTerminationWait))
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
54
internal/adapters/subprocess/execution_test.go
Normal file
54
internal/adapters/subprocess/execution_test.go
Normal file
@@ -0,0 +1,54 @@
|
|||||||
|
package subprocess
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"os/exec"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func TestTerminateAndReapPreservesWaitAndCleanupErrors(t *testing.T) {
|
||||||
|
waitErr := errors.New("wait failed")
|
||||||
|
gracefulErr := errors.New("graceful termination failed")
|
||||||
|
forcefulErr := errors.New("forceful termination failed")
|
||||||
|
tree := &recordingProcessTree{
|
||||||
|
gracefulErr: gracefulErr,
|
||||||
|
forcefulErr: forcefulErr,
|
||||||
|
}
|
||||||
|
waitCh := make(chan error, 1)
|
||||||
|
waitCh <- waitErr
|
||||||
|
|
||||||
|
gotWaitErr, gotCleanupErr := terminateAndReap(tree, waitCh)
|
||||||
|
if !errors.Is(gotWaitErr, waitErr) {
|
||||||
|
t.Fatalf("wait error = %v, want %v", gotWaitErr, waitErr)
|
||||||
|
}
|
||||||
|
for _, want := range []error{gracefulErr, forcefulErr} {
|
||||||
|
if !errors.Is(gotCleanupErr, want) {
|
||||||
|
t.Fatalf("cleanup error = %v, want wrapped %v", gotCleanupErr, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if tree.gracefulCalls != 1 || tree.forcefulCalls != 1 {
|
||||||
|
t.Fatalf("termination calls = graceful %d, forceful %d; want 1 each", tree.gracefulCalls, tree.forcefulCalls)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
type recordingProcessTree struct {
|
||||||
|
gracefulErr error
|
||||||
|
forcefulErr error
|
||||||
|
|
||||||
|
gracefulCalls int
|
||||||
|
forcefulCalls int
|
||||||
|
}
|
||||||
|
|
||||||
|
func (*recordingProcessTree) Start(*exec.Cmd) error { return nil }
|
||||||
|
|
||||||
|
func (tree *recordingProcessTree) TerminateGracefully() error {
|
||||||
|
tree.gracefulCalls++
|
||||||
|
return tree.gracefulErr
|
||||||
|
}
|
||||||
|
|
||||||
|
func (tree *recordingProcessTree) TerminateForcefully() error {
|
||||||
|
tree.forcefulCalls++
|
||||||
|
return tree.forcefulErr
|
||||||
|
}
|
||||||
|
|
||||||
|
func (*recordingProcessTree) Dispose() error { return nil }
|
||||||
Reference in New Issue
Block a user