Skip to content

Close the stdin handle when an exec process exits - #2441

Open
costajohnt wants to merge 1 commit into
apple:mainfrom
costajohnt:fix/exec-stdin-fd-leak
Open

costajohnt wants to merge 1 commit into
apple:mainfrom
costajohnt:fix/exec-stdin-fd-leak

Conversation

@costajohnt

Copy link
Copy Markdown
Contributor

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update

Motivation and Context

Fixes #2439.

startExecProcess closes the exec's stdout and stderr handles once the process exits, but not stdin. Those handles come from XPCMessage.fileHandle(key:), which xpc_fd_dups the descriptor and wraps it with closeOnDealloc: false, and Containerization only reads stdin through the ReaderStream conformance, so nothing else closes it. Every container exec -i therefore leaves one PIPE open in container-runtime-linux.

This closes stdin in the same place, after stdout and stderr. It uses FileHandle.close() rather than closeHandle (raw close(2)) because the stdin relay's readabilityHandler can still be installed when the process exits, for example yes | container exec -i c head -1. Raw-closing the descriptor while that dispatch source is live can run a pending callback against a closed (or reused) fd. In a standalone harness that mirrors this setup, availableData threw NSFileHandleOperationException and aborted the process. FileHandle.close() defers the real close until the source is cancelled.

Related: #2060 and #1928 also touch exec handle cleanup. This change is independent of both and stays correct if #1928 switches these handles to closeOnDealloc: true, since FileHandle.close() marks the handle closed.

Testing

  • Tested locally
  • Added/updated tests
  • Added/updated docs

Ran the reporter's repro.py from #2439 against an alpine:3.20 container, with make all debug builds of main (c6ca9a4) and this branch, each started with container system start --app-root <dir>:

build 8x exec 8x exec -i 8x exec -i after 5s idle
main +0 fds +8 fds +8 fds 16 PIPEs
this branch +0 fds +0 fds +0 fds 0 PIPEs

Also ran yes | container exec -i fdtest head -1 30 times on this branch: the runtime stayed up with 0 PIPEs open. There's no test target for the Linux runtime helper today, so no automated test is added.

startExecProcess closed the stdout and stderr handles after the process
exited but not stdin, so every `container exec -i` left one dup'd PIPE
descriptor open in container-runtime-linux.

stdin is closed with FileHandle.close() rather than close(2): the stdin
relay's readabilityHandler can still be live when the process exits, and
FileHandle.close() waits for that dispatch source to cancel before it
closes the descriptor.

Fixes apple#2439

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: container exec -i leaks one host PIPE descriptor per completed invocation (1.6.1)

1 participant