Preserve validation cancellation during reader shutdown

This commit is contained in:
2026-10-09 03:06:06 +00:00
parent 15dd58f5ac
commit 6eec022bb0
4 changed files with 69 additions and 7 deletions

View File

@@ -54,7 +54,8 @@ neighboring files. Each tree owns one lazy Git blob reader; direct tree users
must close it when finished. The shared validation executor closes its tree on must close it when finished. The shared validation executor closes its tree on
every return before product checks or build callbacks run. A metadata-only every return before product checks or build callbacks run. A metadata-only
selection never starts the reader. Per-read contexts cancel an in-flight blob selection never starts the reader. Per-read contexts cancel an in-flight blob
exchange without tying the reader lifetime to a short-lived caller context. exchange. The context supplied when creating the tree governs the reader
lifetime, including shutdown and child reaping.
Regular-file reads return independent caller-owned byte slices and are not Regular-file reads return independent caller-owned byte slices and are not
retained by the tree. Symlink targets are retained as immutable owned strings retained by the tree. Symlink targets are retained as immutable owned strings
by blob object ID; explicitly reading a symlink still returns independent by blob object ID; explicitly reading a symlink still returns independent

View File

@@ -1,6 +1,6 @@
# Performance Implementation Remediation # Performance Implementation Remediation
**Status:** Required follow-up; remediation pending. **Status:** Remediation in progress; R1 complete.
**Review baseline:** df064bd, reviewed on 2026-10-09 against the **Review baseline:** df064bd, reviewed on 2026-10-09 against the
[performance target](performance.md) and [performance target](performance.md) and
@@ -59,6 +59,12 @@ shutdown is still blocked, and verify completion, child cleanup, and preserved
cancellation identity. Retain coverage of read cancellation, successful closure, cancellation identity. Retain coverage of read cancellation, successful closure,
and preservation of an earlier check failure. and preservation of an earlier check failure.
**Completion:** Complete. The tree retains its operation context for the reader
lifetime. An EOF-synchronized regression now verifies shutdown cancellation,
cancellation identity, and child reaping; it failed before the fix and passes
afterward. Race tests passed for validation, Git, and process packages, including
existing read cancellation, successful closure, and primary-failure coverage.
## R2: Attribute Shutdown Failures To A Check That Read Content ## R2: Attribute Shutdown Failures To A Check That Read Content
**Priority:** Medium. **Priority:** Medium.

View File

@@ -13,6 +13,7 @@ import (
// CandidateTree is an inventory of one captured commit with blob contents // CandidateTree is an inventory of one captured commit with blob contents
// loaded lazily by object ID. // loaded lazily by object ID.
type CandidateTree struct { type CandidateTree struct {
context context.Context
repositoryDir string repositoryDir string
entries []git.TreeEntry entries []git.TreeEntry
byPath map[string]int byPath map[string]int
@@ -24,13 +25,14 @@ type CandidateTree struct {
} }
// ReadCandidateTree enumerates the captured commit without reading from the // ReadCandidateTree enumerates the captured commit without reading from the
// worktree. // worktree. The supplied context also governs the scoped reader through closure.
func ReadCandidateTree(ctx context.Context, repositoryDir, commit string) (*CandidateTree, error) { func ReadCandidateTree(ctx context.Context, repositoryDir, commit string) (*CandidateTree, error) {
entries, err := git.ReadTree(ctx, repositoryDir, commit) entries, err := git.ReadTree(ctx, repositoryDir, commit)
if err != nil { if err != nil {
return nil, err return nil, err
} }
tree := &CandidateTree{ tree := &CandidateTree{
context: ctx,
repositoryDir: repositoryDir, repositoryDir: repositoryDir,
entries: entries, entries: entries,
byPath: make(map[string]int, len(entries)), byPath: make(map[string]int, len(entries)),
@@ -104,10 +106,12 @@ func (tree *CandidateTree) readBlob(ctx context.Context, entry git.TreeEntry) ([
return nil, fmt.Errorf("read committed path %q: candidate tree is closed", entry.Path) return nil, fmt.Errorf("read committed path %q: candidate tree is closed", entry.Path)
} }
if tree.blobReader == nil { if tree.blobReader == nil {
// The reader belongs to the tree and can outlive an individual read // The operation context governs shutdown independently of each read.
// context. Each Read call still applies its own cancellation to the operationContext := tree.context
// in-flight exchange. if operationContext == nil {
tree.blobReader = git.NewBlobReader(context.Background(), tree.repositoryDir) operationContext = context.Background()
}
tree.blobReader = git.NewBlobReader(operationContext, tree.repositoryDir)
} }
contents, err := tree.blobReader.Read(ctx, entry.ObjectID) contents, err := tree.blobReader.Read(ctx, entry.ObjectID)
if err != nil { if err != nil {

View File

@@ -7,7 +7,9 @@ import (
"os" "os"
"os/exec" "os/exec"
"path/filepath" "path/filepath"
"strconv"
"strings" "strings"
"syscall"
"testing" "testing"
"time" "time"
) )
@@ -93,6 +95,55 @@ exec sleep 60`)
} }
} }
func TestRunPreservesCancellationDuringReaderShutdown(t *testing.T) {
repository := newTreeRepository(t)
writeTreeFile(t, repository, "go.mod", []byte("module fixture.example/app\n\ngo 1.25.0\n"))
writeTreeFile(t, repository, "main.go", []byte("package app\n\nconst value = 1\n"))
commit := commitCandidateTree(t, repository)
ready := filepath.Join(t.TempDir(), "shutdown-ready")
installLifecycleTools(t, `while IFS= read -r object; do
size=$("$RELEASEKIT_REAL_GIT" -C "$PWD" cat-file -s "$object")
printf '%s blob %s\n' "$object" "$size"
"$RELEASEKIT_REAL_GIT" -C "$PWD" cat-file blob "$object"
printf '\n'
done
printf '%s\n' "$$" > "$RELEASEKIT_GIT_READER_READY"
exec sleep 60`)
t.Setenv("RELEASEKIT_GIT_READER_READY", ready)
ctx, cancel := context.WithCancel(t.Context())
defer cancel()
result := make(chan error, 1)
go func() { result <- Run(ctx, lifecycleRequest(repository, commit)) }()
waitForLifecycleFile(t, ready)
pidBytes, err := os.ReadFile(ready)
if err != nil {
t.Fatal(err)
}
pid, err := strconv.Atoi(strings.TrimSpace(string(pidBytes)))
if err != nil {
t.Fatal(err)
}
child, err := os.FindProcess(pid)
if err != nil {
t.Fatal(err)
}
cancel()
select {
case err := <-result:
var failure *Failure
if !errors.As(err, &failure) || failure.Check != GoFormat || !errors.Is(err, context.Canceled) {
t.Fatalf("Run() error = %v, want GoFormat cancellation cause", err)
}
if err := child.Signal(syscall.Signal(0)); !errors.Is(err, syscall.ESRCH) && !errors.Is(err, os.ErrProcessDone) {
t.Fatalf("child signal after Run() = %v, want reaped child", err)
}
case <-time.After(5 * time.Second):
_ = child.Kill()
<-result
t.Fatal("Run() did not return after cancellation during reader shutdown")
}
}
func lifecycleRequest(repository, commit string) Request { func lifecycleRequest(repository, commit string) Request {
return Request{ return Request{
RepositoryDir: repository, RepositoryDir: repository,