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

fix: incorrect parsing of closeFDs option #580

Merged
merged 2 commits into from
Feb 13, 2023
Merged

Conversation

deepak1556
Copy link
Contributor

Refs microsoft/vscode-remote-release#7964

When running strace on the scenarios mentioned in the above issue, the spawn helper was spending a lot of time on closing the inherited file descriptors

struct rlimit rlim_ofile;
getrlimit(RLIMIT_NOFILE, &rlim_ofile);
for (rlim_t fd = STDERR_FILENO + 1; fd < rlim_ofile.rlim_cur; fd++) {
if (fd != COMM_PIPE_FD) {
close(fd);
}
}

VSCode does not use the closeFDs option but it was being enabled because of incorrect positional parsing and this PR addresses that.

Additionally, if VSCode were to use closeFDs option we will hit the above issue as in most cases the soft limit of file handles can easily be in the 100k range. A better approach would be to first check if /proc/pid/fd is available in the child process and only restrict to closing the descriptors from this location, fallback can be to loop on the rlimit. I will address this in a follow-up PR.

@Tyriar Tyriar added this to the 1.0.0 milestone Feb 13, 2023
@deepak1556 deepak1556 merged commit aff2e43 into main Feb 13, 2023
@deepak1556 deepak1556 deleted the robo/fix_eage_close_fd branch February 13, 2023 15:59
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.

2 participants