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

guard process creation with a mutex #25

Merged
merged 2 commits into from
Apr 2, 2022
Merged

guard process creation with a mutex #25

merged 2 commits into from
Apr 2, 2022

Conversation

nico
Copy link
Contributor

@nico nico commented Apr 2, 2022

Fixes an fd leak on all unix systems that don't have pipe2.
As of today, that's all unix systems that are not dragonfly bsd,
freebsd, linux, netbsd, openbsd, or redox -- in particular, macOS.

Fixes #14.

@nico nico mentioned this pull request Apr 2, 2022
@nico nico force-pushed the mutex branch 5 times, most recently from 712e861 to b447471 Compare April 2, 2022 02:27
Fixes an fd leak on all unix systems that don't have pipe2.
As of today, that's all unix systems that are not dragonfly bsd,
freebsd, linux, netbsd, openbsd, or redox -- in particular, macOS.

Fixes evmar#14.
@evmar
Copy link
Owner

evmar commented Apr 2, 2022

Thanks! I was thinking that maybe we could use some scoped object (like Runner) to hold the mutex to avoid lazy_static, but I don't think it matters too much either way.

@evmar evmar merged commit 4eb59a6 into evmar:main Apr 2, 2022
@nico nico deleted the mutex branch April 2, 2022 20:31
@nico
Copy link
Contributor Author

nico commented Apr 3, 2022

I was thinking that maybe we could use some scoped object (like Runner) to hold the mutex to avoid lazy_static

Something that could conceivably happen then is that the test suite might create several Runners in parallel (maybe tests run in parallel?) and then that'd still leak FDs. Maybe it's unlikely to happen, or maybe not enough FDs would leak in tests that it really matters though.

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.

n2 leaks fds on macOS
2 participants