Bug: Job log temp file (`tmpFile`) is never closed before deletion in `JobRunner`
- 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
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