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

[Bug]: Fix Wasm executors to exit after run completion #2917

Open
YJDoc2 opened this issue Sep 18, 2024 · 4 comments
Open

[Bug]: Fix Wasm executors to exit after run completion #2917

YJDoc2 opened this issue Sep 18, 2024 · 4 comments
Assignees
Labels

Comments

@YJDoc2
Copy link
Collaborator

YJDoc2 commented Sep 18, 2024

Bug Description

In youki we expect that executor.exec will not return if the exec is successful. This behavior is consistent for tradition containers, where we do an exec syscall and replace process image, thus never returning from exec call. However for wasm workloads our exec implementation does return, and causes crash as it reaches the unreachable! statement. We can either allow the executor to return, or just do a process.exit with exit code from wasm code in our wasm executors to conform it to traditional containers.

Steps to Reproduce

Follow https://containers.github.io/youki/user/webassembly.html to create a wasm container and run it. It will run, but in the end it will error saying -

Printing args
1
2
3
Printing envs
("PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin")
("TERM", "xterm")
("container", "podman")
("HOSTNAME", "b569c0459007")
thread 'main' panicked at crates/libcontainer/src/process/container_init_process.rs:644:5:
internal error: entered unreachable code: the executor should not return if it is successful.
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

Expectation

Youki should not crash in this way. The exit should be graceful, i.e. with a proper exit code (non-zero in case of wasm module returning non-zero exit)

System and Setup Info

N/A , should be reproducible on all systems

Additional Context

FYI, if a new contributor is interested in taking this , ping me (@YJDoc2 ) for help before starting!

@kiokuless
Copy link
Contributor

@YJDoc2 I'm interested in this issue, but I won't be able to work on it until the weekends in October. If that’s okay, I would love to take it on.

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Sep 30, 2024

Hey, sure I will assign this to you.

Context for this is - for normal binaries we use exec syscall so that process image is replcaed by that binary. For wasm, we use wasm runtimes which execute the wasm file. Unlike binary here we do not replcae the process image so after the runtime is done executing, it returns and causes the error. So in the main wasm executor implementation we want to take the exit code returned by those runtime and basically do a process.exit with that exit code.

@kiokuless
Copy link
Contributor

@YJDoc2 Thanks for sharing contexts! My new PC has arrived, I just started using a Linux desktop machine from today. So I can finally tackle this issue. I need an additional bit of time, but I will do this for sure.

@YJDoc2
Copy link
Collaborator Author

YJDoc2 commented Oct 14, 2024

My new PC has arrived, I just started using a Linux desktop machine from today. So I can finally tackle this issue.

Great! Also congrats on the PC and starting with Linux!

I need an additional bit of time, but I will do this for sure.

Yep, no worries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants