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

Use closefrom() to close open file descriptors #712

Merged
merged 1 commit into from
Nov 21, 2024

Conversation

bnoordhuis
Copy link
Contributor

Replace the naive close() loop in js_os_exec with closefrom().

On my system RLIMIT_NOFILE is set to 1 million and the delay from the loop gets noticeable when I spawn many processes.

Fixes: #711

Copy link
Contributor

@saghul saghul left a comment

Choose a reason for hiding this comment

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

Missing include?

@bnoordhuis
Copy link
Contributor Author

I don't think so, it lives in unistd.h but it's hidden behind _DEFAULT_SOURCE on my slightly older ubuntu system. Something probably changed in newer releases.

That said, I discovered musl doesn't have closefrom at all so I'd probably have to dlsym it or use a weak symbol. It'd be nice if something like this could be a simple change for once but we can't have that, can we?

Replace the naive close() loop in js_os_exec with closefrom().

On my system RLIMIT_NOFILE is set to 1 million and the delay from the
loop gets noticeable when I spawn many processes.

Use dlsym() to look up closefrom() because it's not available everywhere
(looking at you, musl.)

Fixes: quickjs-ng#711
@bnoordhuis bnoordhuis merged commit 81e50f3 into quickjs-ng:master Nov 21, 2024
51 checks passed
@bnoordhuis bnoordhuis deleted the fix711 branch November 21, 2024 19:43
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.

Bad js_os_exec performance with high file descriptor limit
2 participants