Skip to content

Close leaked file handles in container config, CRIU stats, and playbook read#28724

Merged
baude merged 1 commit into
containers:mainfrom
SebTardif:fix/close-leaked-file-handles
May 19, 2026
Merged

Close leaked file handles in container config, CRIU stats, and playbook read#28724
baude merged 1 commit into
containers:mainfrom
SebTardif:fix/close-leaked-file-handles

Conversation

@SebTardif
Copy link
Copy Markdown
Contributor

@SebTardif SebTardif commented May 17, 2026

What does this PR do?

Adds missing defer Close() calls in four locations where file handles are opened but never closed:

  1. libpod/container.go (specFromState): os.Open(c.state.ConfigPath) reads the container config but the file handle f is never closed, leaking one fd per call. Introduced in apparmor: apply default profile at container initialization #2114 (2019-01-09).

  2. libpod/container_internal_common.go (checkpoint): os.Open(c.bundlePath()) opens the bundle directory for CRIU dump statistics but the statsDirectory handle is never closed, leaking one fd per checkpoint operation. Introduced in Add support for FreeBSD containers #15632 (2022-08-27).

  3. libpod/container_internal_common.go (restore): Same pattern as checkpoint; statsDirectory opened for CRIU restore statistics is never closed. Introduced in Add support for FreeBSD containers #15632 (2022-08-27).

  4. pkg/machine/shim/host.go (Init): os.Open(opts.PlaybookPath) reads the playbook file but the handle f is never closed after io.ReadAll. Introduced in Add machine init --playbook #25043 (2025-01-17).

How was this tested?

These are mechanical defer Close() additions following the same pattern as #28723 (which was accepted for the same class of fix in pkg/trust/registries.go). The functions involved require complex runtime state (Container with CRIU, machine providers) that makes isolated unit testing impractical. Requesting the No New Tests label.

Release note

Fixed file handle leaks in container config parsing (`specFromState`), CRIU checkpoint/restore statistics, and machine playbook reading.

…ok read

Add missing defer Close() calls in four locations:

- libpod/container.go: specFromState() opens the container config file
  but never closes it after reading, leaking one fd per call.

- libpod/container_internal_common.go: checkpoint() and restore() each
  open the bundle directory for CRIU statistics but never close it,
  leaking one fd per checkpoint/restore operation.

- pkg/machine/shim/host.go: Init() opens the playbook file but never
  closes it after ReadAll, leaking one fd per machine init.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
Copy link
Copy Markdown
Member

@Honny1 Honny1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

Adding label No New Tests and retriggering CI.

@Honny1 Honny1 added the No New Tests Allow PR to proceed without adding regression tests label May 18, 2026
Copy link
Copy Markdown
Member

@baude baude left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@baude baude added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. and removed do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. labels May 19, 2026
@baude baude merged commit 3c4fb92 into containers:main May 19, 2026
86 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

machine No New Tests Allow PR to proceed without adding regression tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants