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
Also create epoll and pipe fds with close-on-exec (#155) #219
Conversation
8995176
to
64beda8
Compare
I can confirm that this fixes a problem I see, too. Bit surprised this wasn't fixed in #178. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Is there anything preventing this fix from being merged (other than needing a re-base on master)? This PR is from 2017 but the issue of these FDs leaking across execs is still present today. |
64beda8
to
df57317
Compare
I've rebased the patch onto current master, everything still seems to work fine. It would be good to have it merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
* Add the unix.O_CLOEXEC to the Pipe2 call * Add unix.EPOLL_CLOEXEC to the Epoll call
df57317
to
92451cc
Compare
I just force-pushed a no-op change to try and re-trigger the CI. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ugh 😞 CI issues due to golint not working on go1.8 anymore. |
Finally, all green. Merging. Thanks! 🎉 ❤️ |
🎉 |
I've signed the CLA.
What does this pull request do?
When creating a fsnotify.NewWatcher(), four file descriptors are opened: One for inotify, one for epoll and two for the two ends of a pipe. Only the inotify one has CLOEXEC set. This PR adds the CLOEXEC flag to all four file descriptors, preventing a leak in cases where execve is used e.g. to reload an executable on code change.
Where should the reviewer start?
I read the manpages of pipe2 and epoll_create2 to get an idea of the flags.
How should this be manually tested?
In a simple program which just opens a NewWatcher, does a syscall.Exec, and sleeps, the difference in the number of open file descriptors is visible in /proc//fd/