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

Reconsider if closing pre-opens are prohibited #50

Open
codefromthecrypt opened this issue Jan 31, 2023 · 5 comments
Open

Reconsider if closing pre-opens are prohibited #50

codefromthecrypt opened this issue Jan 31, 2023 · 5 comments

Comments

@codefromthecrypt
Copy link

@loganek recently added a rust close_preopen test , which ensures user code can't close a.. pre-open. In some ways stdio are like pre-opens, as in wazero, we had exactly the opposite request by @vyskocilm which is to ensure stdio can be closed. I'd like to understand how to rationalize both at the same time, even if wazero tentatively blocks closing strict pre-opens (fd=3) while still allowing code to close stdio (implicit pre-opens)

See tetratelabs/wazero#953

codefromthecrypt pushed a commit to tetratelabs/wazero that referenced this issue Jan 31, 2023
See WebAssembly/wasi-testsuite#50

Signed-off-by: Adrian Cole <adrian@tetrate.io>
codefromthecrypt added a commit to tetratelabs/wazero that referenced this issue Jan 31, 2023
See WebAssembly/wasi-testsuite#50

Signed-off-by: Adrian Cole <adrian@tetrate.io>
sunfishcode added a commit to sunfishcode/WASI that referenced this issue Feb 24, 2023
Add documentation mentioning stdin, stdout, and stderr file descriptors,
and also preopen file descriptors, and mention that preopen file
descriptors can be closed.

Allowing preopen file descriptors to be closed is a change from how
Wasmtime historically worked, but I think it's more clear now that
it's ok to let preopens be closed. We'll change Wasmtime accordingly.

This addresses WebAssembly/wasi-testsuite#50.
@sunfishcode
Copy link
Member

Your reasoning makes sense to me. We should change the spec to allow preopens to be closed. I've now filed WebAssembly/WASI#522 to propose this.

sunfishcode added a commit to WebAssembly/WASI that referenced this issue Mar 6, 2023
* Add documentation about starting file descriptors in Preview1.

Add documentation mentioning stdin, stdout, and stderr file descriptors,
and also preopen file descriptors, and mention that preopen file
descriptors can be closed.

Allowing preopen file descriptors to be closed is a change from how
Wasmtime historically worked, but I think it's more clear now that
it's ok to let preopens be closed. We'll change Wasmtime accordingly.

This addresses WebAssembly/wasi-testsuite#50.

* Say "stream" instead of "file-like".
@david32768
Copy link

Suppose preopens are 3, 4 and 5 and 4 is closed and a new scan of preopens is done then 5 will not be found.

1 similar comment
@david32768
Copy link

Suppose preopens are 3, 4 and 5 and 4 is closed and a new scan of preopens is done then 5 will not be found.

@codefromthecrypt
Copy link
Author

Suppose preopens are 3, 4 and 5 and 4 is closed and a new scan of preopens is done then 5 will not be found.

Interesting, I had noticed that pre-open scans tended to be a one-shot deal (at program init), can you link me something that does this routinely? Something that does could help make a test to ensure they continue to work.

@david32768
Copy link

I just want clarity. It is a pity that obtaining predirs is not like args_get and environment_get then the fd's need not be 3,4,...

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

3 participants