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

Don't error if guest calls proc_exit with exit code 0 #323

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

itowlson
Copy link
Contributor

@itowlson itowlson commented Apr 6, 2022

Fixes #321.

The issue, as discussed there, is that Zig build-exe generates a module that calls WASI proc_exit. This is realised in Wasmtime as a trap, and any trap is realised in Rust as an error. This PR rather recovers the trap and checks whether it was a benign exit.

This needs to be backported to WAGI. I think it's not needed for the Spin executor but would appreciate other folks' thoughts on that!

@itowlson itowlson requested a review from radu-matei April 6, 2022 06:32
@@ -110,7 +110,7 @@ impl HttpExecutor for WagiHttpExecutor {
// even if the guest code fails. (And when checking, check the guest
// result first, so that guest failures are returned in preference to
// log failures.)
guest_result??;
ignore_successful_proc_exit_trap(guest_result?)?;
Copy link
Collaborator

@lann lann Apr 6, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps slightly Rustier equivalent:

guest_result?.or_else(ignore_successful_proc_exit_trap)?;
...
fn ignore_successful_proc_exit_trap(guest_err: anyhow::Error) -> Result<()> {
  match guest_err.root_cause()...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

Copy link
Collaborator

@lann lann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One non-blocking suggestion

@technosophos
Copy link
Contributor

Wha! Good find! That is not what I would have expected to be the root of Zig's error.

@itowlson itowlson merged commit 0a57f9f into fermyon:main Apr 6, 2022
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 this pull request may close these issues.

Zig binaries that work with Wasmtime do not work with Spin
4 participants