buildkite / buildkite/agent

Bug: Job log temp file (`tmpFile`) is never closed before deletion in `JobRunner`

Open Beginner friendly
#4,331 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
1.1k
Forks
378
Avg merge
2d 6h
Merged PRs (30d)
74

Description

## Describe the bug
In `agent/job_runner.go`, when the agent option `enable-job-log-tmpfile` is enabled, the temporary file created via `os.CreateTemp` is never closed (`tmpFile.Close()`).

When the job process completes, a cleanup goroutine calls `os.Remove(tmpFile.Name())` while the file descriptor remains open.

This leads to two distinct failures:
1. **On Windows**: `os.Remove` fails with `The process cannot access the file because it is being used by another process`, leaving orphaned temporary files accumulating in the temp directory.
2. **On Linux/macOS**: `os.Remove` unlinks the path, but the file descriptor is leaked in the agent process. Long-running agents executing many jobs will accumulate open file descriptors until hitting system limits (`too many open files`).

## Steps To Reproduce
1. Run the agent with `--enable-job-log-tmpfile` enabled (or set `enable-job-log-tmpfile: true` in the configuration).
2. Execute one or more jobs through the agent runner.
3. Observe process file descriptors or check the agent logs on Windows after job completion.

## Expected behavior
When a job process finishes, the temporary log file should be closed (`tmpFile.Close()`) before being removed (`os.Remove(tmpFile.Name())`).

Example execution command:
```bash
./buildkite-agent bootstrap \
--enable-job-log-tmpfile \
--build-path=/tmp/bk-builds --job demo --phases command \
--repository . --commit HEAD --branch main --pipeline-provider custom \
--agent a --organization o --pipeline p \
--command 'echo "hello"'
Actual behaviour
tmpFile is deleted without being closed first. On Windows, os.Remove fails silently or logs Couldn't remove job log temp file: ... and leaves the temp file on disk. On Unix systems, file descriptors leak in the main agent process.

Stack parameters (please complete the following information)
Version: v3.x / v4.x (main branch)

Additional context
The cleanup logic in agent/job_runner.go (around line 339) is:

go
go func() {
<-r.process.Done()
if tmpFile != nil {
// tmpFile.Close() is missing here
if err := os.Remove(tmpFile.Name()); err != nil {
r.agentLogger.Errorf("Couldn't remove job log temp file: %v", err)
}
}
}()

Contributor guide

Open the contributing guide

Research direction

Start in agent/job_runner.go around the cleanup goroutine near line 339 and trace how tmpFile is created and removed. Use the provided enable-job-log-tmpfile execution command to observe cleanup on the supported platforms. Done means the temporary file is closed before removal, with no leaked descriptor or Windows cleanup failure.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
ci-cd, devops
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.