Bound subprocess output teardown

This commit is contained in:
2026-11-02 03:14:59 +00:00
parent 94aedacf05
commit 9c02ed769b
4 changed files with 101 additions and 10 deletions

View File

@@ -39,7 +39,7 @@ the agent policy; an intermediate compilation dependency is recorded accurately.
| 1 | A01; publication assertion portion of A19 | Complete — regression failed before the fix and passed afterward; focused publication and reconciliation tests passed. |
| 2 | A02, A12: common Git mechanics | Complete — focused Git, candidate, identity, and public build/publication/delivery tests passed; A02 regression evidence follows in Stage 3. |
| 3 | A02: source identity regression evidence | Complete — commit, tree, and blob replacement regressions and public publication, build, and delivery cases passed. |
| 4 | A03: buffered command teardown | Pending |
| 4 | A03: buffered command teardown | Complete — inherited-pipe, cancellation, healthy-command, native build, and publication recovery checks passed. |
| 5 | A03, A18: piped lifetime and queued cancellation | Pending |
| 6 | A04 | Pending |
| 7 | A05 | Pending |

View File

@@ -2,6 +2,7 @@
package build
import (
"bytes"
"context"
"crypto/sha256"
"encoding/hex"
@@ -9,7 +10,6 @@ import (
"fmt"
"io"
"os"
"os/exec"
"path/filepath"
"strings"
@@ -17,6 +17,7 @@ import (
"go.maximumdirect.net/releasekit/internal/candidate"
"go.maximumdirect.net/releasekit/internal/git"
"go.maximumdirect.net/releasekit/internal/gotool"
"go.maximumdirect.net/releasekit/internal/process"
"go.maximumdirect.net/releasekit/internal/releaseidentity"
"go.maximumdirect.net/releasekit/internal/tempdir"
"go.maximumdirect.net/releasekit/internal/versioncontract"
@@ -227,17 +228,14 @@ func hashArtifact(artifact *Artifact) error {
}
func checkVersion(ctx context.Context, binaryPath, binaryName, version string) error {
command := exec.CommandContext(ctx, binaryPath, "version")
output, err := command.CombinedOutput()
var output bytes.Buffer
err := process.Run(ctx, process.Invocation{Args: []string{binaryPath, "version"}, Stdout: &output, Stderr: &output})
if err != nil {
if ctx.Err() != nil {
return fmt.Errorf("run native version check: %w", ctx.Err())
}
return fmt.Errorf("run native version check: %w: %s", err, strings.TrimSpace(string(output)))
return fmt.Errorf("run native version check: %w: %s", err, strings.TrimSpace(output.String()))
}
want := binaryName + " " + version + "\n"
if string(output) != want {
return fmt.Errorf("native version output = %q, want %q", output, want)
if output.String() != want {
return fmt.Errorf("native version output = %q, want %q", output.String(), want)
}
return nil
}

View File

@@ -11,6 +11,7 @@ import (
"sort"
"strings"
"sync"
"time"
)
// Invocation describes one child process. UnsetEnv removes inherited values
@@ -135,6 +136,7 @@ func newCommand(ctx context.Context, invocation Invocation) (*exec.Cmd, error) {
}
command := exec.CommandContext(ctx, invocation.Args[0], invocation.Args[1:]...)
command.Dir = invocation.Dir
command.WaitDelay = time.Second
if len(invocation.Env) != 0 || len(invocation.UnsetEnv) != 0 {
command.Env = environmentEntries(invocation.Env, invocation.UnsetEnv)
}

View File

@@ -10,6 +10,7 @@ import (
"os"
"os/exec"
"path/filepath"
"strconv"
"strings"
"testing"
"time"
@@ -119,6 +120,68 @@ func TestRunReturnsCancellation(t *testing.T) {
}
}
func TestRunBoundsInheritedOutputPipes(t *testing.T) {
for _, mode := range []string{"cancel-descendant", "exit-descendant"} {
t.Run(mode, func(t *testing.T) {
ready := filepath.Join(t.TempDir(), "descendant")
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
var output bytes.Buffer
result := make(chan error, 1)
go func() {
result <- Run(ctx, Invocation{
Args: []string{helperExecutable(t), "-test.run=TestProcessHelper", "--", "unused", mode},
Env: map[string]string{"RELEASEKIT_PROCESS_HELPER": "1", "RELEASEKIT_DESCENDANT_READY": ready},
Stdout: &output, Stderr: &output,
})
}()
deadline := time.After(10 * time.Second)
var pid int
for pid == 0 {
data, err := os.ReadFile(ready)
if err == nil {
pid, _ = strconv.Atoi(string(data))
}
select {
case <-deadline:
t.Fatal("descendant did not become ready")
default:
time.Sleep(10 * time.Millisecond)
}
}
t.Cleanup(func() {
if process, err := os.FindProcess(pid); err == nil {
_ = process.Kill()
}
})
if mode == "cancel-descendant" {
cancel()
}
select {
case err := <-result:
if mode == "cancel-descendant" && !errors.Is(err, context.Canceled) {
t.Fatalf("canceled command error = %v", err)
}
if mode == "exit-descendant" && !errors.Is(err, exec.ErrWaitDelay) {
t.Fatalf("exited command error = %v, want pipe teardown error", err)
}
case <-time.After(10 * time.Second):
t.Fatal("command did not release inherited output pipes")
}
})
}
}
func TestRunDoesNotLimitHealthyCommandLifetime(t *testing.T) {
err := Run(context.Background(), Invocation{
Args: []string{helperExecutable(t), "-test.run=TestProcessHelper", "--", "unused", "slow-success"},
Env: map[string]string{"RELEASEKIT_PROCESS_HELPER": "1"},
})
if err != nil {
t.Fatalf("healthy command failed: %v", err)
}
}
func TestStartPipedExchangesBeforeInputEOFAndPreservesSettings(t *testing.T) {
workdir := filepath.Join(t.TempDir(), "piped directory")
if err := os.Mkdir(workdir, 0o700); err != nil {
@@ -279,11 +342,39 @@ func TestProcessHelper(t *testing.T) {
os.Exit(0)
case "exit":
os.Exit(7)
case "slow-success":
time.Sleep(1500 * time.Millisecond)
os.Exit(0)
case "wait":
fmt.Fprintln(os.Stdout, "started")
for {
time.Sleep(time.Second)
}
case "cancel-descendant", "exit-descendant":
child := exec.Command(os.Args[0], "-test.run=TestProcessHelper", "--", "unused", "descendant")
child.Stdout, child.Stderr = os.Stdout, os.Stderr
if err := child.Start(); err != nil {
os.Exit(14)
}
for {
if _, err := os.Stat(os.Getenv("RELEASEKIT_DESCENDANT_READY")); err == nil {
break
}
time.Sleep(10 * time.Millisecond)
}
if mode == "exit-descendant" {
os.Exit(0)
}
for {
time.Sleep(time.Second)
}
case "descendant":
if err := os.WriteFile(os.Getenv("RELEASEKIT_DESCENDANT_READY"), []byte(strconv.Itoa(os.Getpid())), 0o600); err != nil {
os.Exit(15)
}
for {
time.Sleep(time.Second)
}
case "exchange":
reader := bufio.NewReader(os.Stdin)
request, err := reader.ReadString('\n')