lunatic icon indicating copy to clipboard operation
lunatic copied to clipboard

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

Open MarkintoshZ opened this issue 2 years ago • 5 comments

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.

https://github.com/lunatic-solutions/lunatic/blob/1ba1c75accdb5fedbbaa8ea285204024bccc6012/crates/lunatic-wasi-api/src/lib.rs#L22

Is there a reason why preopening CWD is necessary?

MarkintoshZ avatar Jun 01 '22 03:06 MarkintoshZ

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.

bkolobara avatar Jun 02 '22 11:06 bkolobara

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);

MarkintoshZ avatar Jun 03 '22 19:06 MarkintoshZ

Yes, feel free to submit a PR.

bkolobara avatar Jun 03 '22 19:06 bkolobara

Sounds good.

MarkintoshZ avatar Jun 03 '22 19:06 MarkintoshZ

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 avatar Jun 14 '22 16:06 bkolobara