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

Opening preopened_dirs repeatedly with each process spawn exceeds maximum file descriptor limit #104

Open
MarkintoshZ opened this issue Jun 1, 2022 · 5 comments

Comments

@MarkintoshZ
Copy link
Contributor

The unit test process::recursive_count in rust-lib fails on my laptop running macOS Catalina. The cause of is issue is that the current working directory is opened repeatedly with each process spawn. With the default maximum file descriptor limit of 256, the unit test with 1000 recursion calls exceeds the limit easily.

let preopen_dir = Dir::open_ambient_dir(preopen_dir_path, ambient_authority())?;

Is there a reason why preopening CWD is necessary?

@bkolobara
Copy link
Contributor

Thank you @MarkintoshZ for looking into this and pinpointing the issue. It would be really hard for me to reproduce it.

We use the wasmtime-wasi crate to expose filesystem operations to the guest. Each process can get a set of files/directories it has access to and by default the main process gets access to the CWD. So all child processes inherit this right. Unfortunately, it looks like wasmtime-wasi will always open the directory and not only lazily if the guest requires access to it during runtime.

Processes will always require access to files and directories and this eager pre-opening could be an issue if you have hundreds of thousands of processes running. I will see if we can maybe fix this directly in wasmtime-wasi.

@MarkintoshZ
Copy link
Contributor Author

Sounds good! Thank you!

In the meanwhile, should we add the following lines to the unit test as a temporary fix?

let mut config = ProcessConfig::new();
config.set_can_spawn_processes(true);
Process::spawn_link_config(&config, (mailbox.this(), 1000), recursive_count_sub);

@bkolobara
Copy link
Contributor

Yes, feel free to submit a PR.

@MarkintoshZ
Copy link
Contributor Author

Sounds good.

@bkolobara
Copy link
Contributor

I will reopen this one, because the underlaying issue is not resolved yet. We shouldn't be pre-opening a folder if the parent process has access to it. Or we should somehow share the FD between processes.

@bkolobara bkolobara reopened this Jun 14, 2022
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

No branches or pull requests

2 participants