github-actions[bot] commented on Issue #1443:
Subscribe to Label Action
This issue or pull request has been labeled: "wasi"
<details> <summary>Users Subscribed to "wasi"</summary>
- @kubkon
</details>
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
kubkon commented on Issue #1443:
@sunfishcode Did you perhaps have a look at the proposed changes in this PR? I'd like to merge this in so that we can move on to other things such as the polyfill, etc.
kubkon commented on Issue #1443:
This looks good!
Handlegrowing into a proper item rather than "weird thing for virtfs" is nice to see :)The hierarchy you mentioned,
\-- OsHandle \-- OsFile -- Stdio \-- Virtualis slightly different from what I'd had in mind, something like:
\-- File \-- Os -- Virtual \-- Stream \-- Os -- Virtualwhere delineating on a
File/not-Fileline seemed to match with "can I even take an action on this handle", likeseekon aStream, where "can this be fulfilled with the handle I have" is possibly an implementation constraint from "isvirtualfsimplemented in all the places people want to use it".To that end, I think it's about the same as you've outlined here, where there's a trait impl that either form takes, so we don't have to worry about nonsense calls like asking
VirtualFilefor its raw fd. I just mention this here because I'm not sure if this is still the same direction you're thinking, or maybe I'd missed an important conversation in the last month or so.I'm also curious about the
RawFdshenanigans we do in wasi-common - is there a particular reason we _need_ to touchRawFdorManuallyDrop? I see there are cases we have to update a referenced file descriptor (fdstat_set_flags, to support Windows' impl), but even the we could retain the returnedFilerather than swapping outfdnumbers under the hood. The implementation here looks good, but it'd be nice to be sure we won't accidentally pick up a bug in the future from some change that forgets to drop anfd:DApproving as this all looks good, and even if there are points to keep from my comment they can be changes in a subsequent PR or here - your choice.
So actually, your model is exactly what I had in mind here. Perhaps naming is somewhat misleading here. When refactoring, I noticed that
VirtualFsis likestd::fs::File, hence why I pushed it out fromOsHandlesinceOsHandleis also meant to act likestd::fs::File. And when streams come up, they will be a type separate fromOsHandle. Perhaps renaming will be in order then to avoid this sort of confusion!
kubkon commented on Issue #1443:
So after some offline discussion with @sunfishcode, we agreed to land this PR with the intention to take this refactoring further however by splitting the "one trait" for WASI handle into multiple traits, each responsible for different type of functionality, essentially disambiguating between files, dirs, streams, etc. This will take some considerable amount of effort to pull off, but anyway, we do believe this PR is a good first step in that direction, and landing it will give us some decent base to iterate from.
Last updated: Dec 13 2025 at 19:03 UTC