Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent fd leaks to the running shell or command #5341

Merged
merged 1 commit into from
Jan 24, 2025
Merged

Conversation

mitchellh
Copy link
Contributor

Multiple fixes to prevent file descriptor leaks:

  • libxev eventfd now uses CLOEXEC
  • linux: cgroup clone now uses CLOEXEC for the cgroup fd
  • termio pipe uses pipe2 with CLOEXEC
  • pty master always sets CLOEXEC because the child doesn't need it
  • termio exec now closes pty slave fd after fork

There still appear to be some fd leaks happening. They seem related to GTK, they aren't things we're accessig directly. I still want to investigate them but this at least cleans up the major sources of fd leakage.

@mitchellh mitchellh force-pushed the push-lruzutnwtpqr branch 2 times, most recently from 2a0b8f2 to 6eee0b9 Compare January 24, 2025 06:07
@mitchellh mitchellh enabled auto-merge January 24, 2025 06:09
Multiple fixes to prevent file descriptor leaks:

- libxev eventfd now uses CLOEXEC
- linux: cgroup clone now uses CLOEXEC for the cgroup fd
- termio pipe uses pipe2 with CLOEXEC
- pty master always sets CLOEXEC because the child doesn't need it
- termio exec now closes pty slave fd after fork

There still appear to be some fd leaks happening. They seem related to
GTK, they aren't things we're accessig directly. I still want to
investigate them but this at least cleans up the major sources of fd
leakage.
@mitchellh mitchellh disabled auto-merge January 24, 2025 06:21
@mitchellh mitchellh merged commit fd8caca into main Jan 24, 2025
58 checks passed
@mitchellh mitchellh deleted the push-lruzutnwtpqr branch January 24, 2025 06:21
@github-actions github-actions bot added this to the 1.1.0 milestone Jan 24, 2025
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.

1 participant