Skip to content

Commit 818d0f3

Browse files
Sayan-masnwilliams
andauthored
Reap the log aggregator's tail processes (#350)
## Summary `tailFile` starts `tail -n +1 -F <path>` and reads its stdout, but never calls `Wait`. When that tail exits, nothing collects its status, so it stays a zombie. The wrapper is pid 1 in both the container (`ENTRYPOINT`) and the unikernel (Kraftfile `cmd`), so there is no other init to clean up after it. On a long-lived instance whose services restart repeatedly, these accumulate for the life of the instance and each one holds a pid slot. A clean scan ends exactly when tail closes its stdout, which is when it has exited, so that is where the wait belongs. A scan that ends on a read error instead is a different case: `bufio.Scanner` also stops on `ErrTooLong`, and there tail is still alive, so waiting would park the goroutine forever. Kill it first in that path. ## Testing `go build`, `go vet`, `gofmt` and `go test -race` pass on `server/cmd/wrapper`. No test added here. `tail -F` does not exit on its own and `tailFile` keeps no handle to the process, so there is no seam to drive it from a test without reshaping the function. The reaper PR stacked on this one carries tests that cover the same defect class directly. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Localized change to log tailing cleanup in the wrapper; no auth, data, or API surface impact. > > **Overview** > **`tailFile`** in the wrapper supervisord log aggregator previously started `tail -F` and read lines but never **`Wait()`** on the child. Because the wrapper runs as **pid 1** in container and unikernel, exited tails became **zombies** that could pile up when services restart. > > After the scan loop, the change always **`Wait()`**s to reap the process. If **`scanner.Err()`** is set (e.g. a line exceeds the 1MB scanner limit while `tail` is still running), it **`Kill()`**s `tail` first so **`Wait()`** does not block forever. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 20a0f55. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> Co-authored-by: Sayan- <1415138+Sayan-@users.noreply.github.com> Co-authored-by: Mason Williams <43387599+masnwilliams@users.noreply.github.com>
1 parent 8c5de5c commit 818d0f3

1 file changed

Lines changed: 11 additions & 0 deletions

File tree

server/cmd/wrapper/supervisord.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,17 @@ func tailFile(path string) {
9797
for scanner.Scan() {
9898
fmt.Printf("[%s] %s\n", label, scanner.Text())
9999
}
100+
// A clean scan ends when tail closes its stdout, i.e. when it has exited.
101+
// A scan that ends on an error (a log line past the 1MB cap, say) leaves
102+
// tail running, so kill it rather than block here forever.
103+
//
104+
// Either way we have to collect it: the wrapper is pid 1 in both the
105+
// container and the unikernel, so a tail nobody waits on stays a zombie
106+
// for the life of the instance.
107+
if scanner.Err() != nil {
108+
_ = cmd.Process.Kill()
109+
}
110+
_ = cmd.Wait()
100111
}
101112

102113
func runStream(label, name string, args ...string) error {

0 commit comments

Comments
 (0)