-
Notifications
You must be signed in to change notification settings - Fork 90
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
feat(wamr): add wasm-mico-runtime shim implementation #508
Conversation
this commit adds an additional shim implementation: Wamr using wamr-rust-sdk Signed-off-by: jiaxiao zhou <[email protected]>
@@ -3,7 +3,7 @@ INSTALL ?= install | |||
CARGO ?= cargo | |||
LN ?= ln -sf | |||
TEST_IMG_NAME ?= wasmtest:latest | |||
RUNTIMES ?= wasmedge wasmtime wasmer | |||
RUNTIMES ?= wasmedge wasmtime wasmer wamr |
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.
Need to add wamr to CI
this is GREAT! |
Signed-off-by: jiaxiao zhou <[email protected]>
let instance = WamrInstnace::new(&module, 1024 * 64) | ||
.map_err(|e| anyhow::Error::msg(format!("Failed to create instance: {:?}", e)))?; | ||
|
||
// TODO: bug: failed at line above saying: `thread signal env initialized failed` |
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.
I am thinking about Self { runtime }
may has been destroyed. It happened several times in my test cases when implementing rust-sdk. Is there a quick way we can confirm that?
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.
yeah i believe that when the variable let runtime = Runtime::new()
goes out of scope, the Drop()
will be invoked to destruct it.
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.
Let's probably why I favor the design of a binding the runtime to the variable and pass that to instances like wasmtime does.
let engine = Engine::default();
let module = Module::new(&engine, wat)?;
let mut store = Store::new(&engine, 4);
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.
about that, bytecodealliance/wamr-rust-sdk#8
@Mossaka does wamr work already? |
WAMR team can provide help as much as possible. |
@Mossaka it would be great if you could check the status of this PR! Keep me posted |
Based on the description in bytecodealliance/wasm-micro-runtime#3627. It requires to config pre-opens of WASI arguments. Here is a reference: https://github.com/bytecodealliance/wamr-rust-sdk/blob/main/examples/wasi-hello/src/main.rs#L19-#L21 |
@lum1n0us please take a look at the PR I do set I have also tried with other dirs, just like in an example, it fails with "Error: ExecutionError("Exception: unreachable")" To clarify: |
Like, may I ask, if using |
The log is containerd's log and containerd is running on your local host as a deamon process. You can try You may also refer to this doc on how to adjust logging levels. Note that you may need to restart containerd after modiying its config files by doing |
Thanks. Please don't mind my lack of related knowledge.
|
If you wanna see logs from runwasi and sdk code, you can put
I've not tried this but I did try attaching the PID to vscode debugger to debug the shim binary.
What do you mean by this exactly? If you wanna create a simply wasm image, you can do so by authoring a hello world program, compile it to wasm, and package it as a OCI image, and then run it with |
Try this command but failed: $ sudo ctr run --rm --runtime=io.containerd.wasmtime.v1 ghcr.io/containerd/runwasi/wasi-demo-app:latest testwasm echo "hi"
ctr: failed to create shim task: Others("exec process failed with error error in executing process : wasmtime executor can't handle spec"): unknown Using
It is similar with #637. Any suggestion? |
anyhow::Error::msg(format!("Failed to create module from bytes: {:?}", e)) | ||
})?; | ||
|
||
module.set_wasi_arg_pre_open_path(vec![String::from("/")], vec![String::from("/")]); |
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.
module.set_wasi_arg_pre_open_path(vec![String::from("/")], vec![String::from("/")]); | |
module.set_wasi_arg_pre_open_path(vec![String::from("/")], vec![]); |
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.
Still have problems when function.call()
. need further investigation. But blocked by above setup issues. I was hoping to check
- if
module
is unloaded or released? - if there anything unexpected unloaded or released?
Try I believe the wasi-demo-app takes the first argument as the wasm file and the second argument as the command. |
There is an exception in wasi-demo-app.wasm when driving by WAMR. It is triggered by After some investigation, I learned why
Therefore,
|
@Mossaka need your help to take a look at below two problems. Both return values in runwasi/containerd are different with other execution environments.
|
A workaround of this issue is to disable memory bounds check with But the reason of this problem still bothers me. |
@Mossaka. Above unanticipated behaviors of sandbox(read-only stdout and guard page failures) still exist. Hope somebody would take a look. We made some workarounds in WAMR and wamr-rust-sdk. #642 should work if following #642 (comment). |
this commit adds an additional shim implementation: Wamr using wamr-rust-sdk. At the moment, the shim isn't working with an error
"thread signal env initialized failed"
which I am not sure exactly sure what it means. Will ask the Wamr team for clarification. There are also some pain points using the newly created wamr-rust-sdk which I wrote as TODOs in the comment.This PR should close #337
FYI @squillace @0xE282B0 @lum1n0us