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

wasi-streams for stdio: Spawned worker threads should not abort when the write end is dropped #7258

Closed
elliottt opened this issue Oct 16, 2023 · 1 comment · Fixed by #7388
Assignees

Comments

@elliottt
Copy link
Member

elliottt commented Oct 16, 2023

Currently the worker thread managing writes to stdout will drop when the guest-side write end is dropped. We need to fix this, as it's quite common to repeatedly call std::io::get_stdout() and drop the result in rust.

The general approach that we discussed is to have the stdout/stderr worker tasks alive for the duration of the guest, and a separate channel for reporting new write channels to those tasks. The tasks would then be responsible for selecting between the new writer channel and all the known writer channels when processing incoming jobs, removing known writers from the managed set when those ends drop instead of aborting.

@elliottt elliottt changed the title wasi-streams: Spawned worker threads should not abort when the read end is dropped wasi-streams: Spawned worker threads should not abort when the write end is dropped Oct 16, 2023
@elliottt elliottt self-assigned this Oct 16, 2023
@pchickey pchickey changed the title wasi-streams: Spawned worker threads should not abort when the write end is dropped wasi-streams for stdio: Spawned worker threads should not abort when the write end is dropped Oct 16, 2023
@alexcrichton
Copy link
Member

@pchickey question for you, do you remember why we have it such that stdout/stderr perform async writes? Was that made as a specific design choice or rather "well everything is async so may as well"?

If the latter I might actually propose solving this issue by "just" doing a blocking write to stdout/stderr. I feel like I'm forgetting something though because I suspect we would have tried to talk about this when figuring out flushing and such. What I'm thinking is basically the implementation of LogStream from @elliottt's current PR is what we'd bake in as the default stdout/stderr (minus the addition of prefixes per request).

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 a pull request may close this issue.

2 participants