-
Notifications
You must be signed in to change notification settings - Fork 925
Implement asyncify-free vforking #5980
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request introduces support for a new proc_vfork syscall that enables vforking without requiring asyncify stack rewinding. The implementation distinguishes between asyncify and non-asyncify vfork execution paths by making the WasiVFork::rewind_stack field optional, allowing the system to handle both cases appropriately.
Key changes:
- Added new
proc_vforksyscall that setsrewind_stacktoNone, enabling vfork without asyncify - Modified
WasiVForkstruct to useOption<BytesMut>forrewind_stackfield instead of requiring it - Updated
proc_exitandproc_exec3to handle non-asyncify vfork paths with early returns whenrewind_stack.is_none()
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/wasix/src/syscalls/wasix/proc_vfork.rs | New syscall implementation for asyncify-free vforking; captures store snapshot but omits rewind stack |
| lib/wasix/src/syscalls/wasix/proc_fork.rs | Updated to wrap rewind_stack in Some() to match the new Option type |
| lib/wasix/src/syscalls/wasi/proc_exit.rs | Added early return path for non-asyncify vfork before unwrapping rewind_stack |
| lib/wasix/src/syscalls/wasix/proc_exec3.rs | Added pattern matching to handle non-asyncify vfork path before attempting stack rewind |
| lib/wasix/src/syscalls/wasix/mod.rs | Registered new proc_vfork module in exports |
| lib/wasix/src/lib.rs | Updated WasiVFork struct documentation and added proc_vfork to 32-bit and 64-bit exports |
| lib/wasix/src/bin_factory/exec.rs | Updated resume_vfork condition to handle non-asyncify case as an error in unexpected exit scenarios |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 16 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
// TODO: Review copilot PR descriptionThis pull request adds support for a new
proc_vforksyscall and refactors the vfork implementation to handle both asyncify and non-asyncify cases more robustly. The changes ensure that vforked processes that do not use asyncify are managed correctly, and the code now distinguishes between these two execution paths. Additionally, the vfork data structure and related syscalls are updated to support this distinction.Vfork syscall support and handling:
proc_vforksyscall implementation inproc_vfork.rs, allowing vfork without stack rewinding (asyncify) and updating the environment and process management accordingly.proc_vforksyscall in both the 32-bit and 64-bit WASIX exports, making it available to WASI modules.mod.rsto include the newproc_vforkimplementation.Vfork data structure and asyncify handling:
WasiVForkstruct'srewind_stackfield to be anOption<BytesMut>, allowing for cases where asyncify is not used.proc_forkto setrewind_stackasSome(...), and updated all vfork-related code paths to handle theOptiontype, including proper handling of the non-asyncify path inproc_exitandproc_exec3.Process management improvements:
Minor improvements:
owned_handlesfor correct resource tracking.These changes collectively improve the robustness and flexibility of process creation and management in WASIX, especially around the vfork syscall and its interaction with asyncify.