Bump Wasmtime to v48.0.0 prerelease - #3653
Conversation
Signed-off-by: spinframeworkbot <202838904+spinframeworkbot@users.noreply.github.com>
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
|
I bumped it to 1.95 and fixed up the low-hanging fruit around filesystem permissions, but there's a big pile of errors around changes to the sockets API that I don't know how to start on. |
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
Signed-off-by: itowlson <ivan.towlson@fermyon.com>
|
I updated the SocketAddrUse stuff based on bytecodealliance/wasmtime#13934, but it needs someone to check it because I am somewhat guessing! A lot of the upstream sockets stuff has changed from sync to async or from async to sync. For some of it I was able to propagate the change without obvious problems, but there are some cases (I think all implementing RCs) where we are constrained by a sync trait but the underlying API is now async, and my usual kludges for bridging over that feel ill-advised. So @alexcrichton or @dicej I think I am going to need to punt those to you - sorry. |
|
Thanks for getting this going, @itowlson! I'm addressing the remaining issues and will push an update. |
2082111 to
7b728be
Compare
| ctx: PrepareContext<T, Self>, | ||
| ) -> anyhow::Result<InstanceBuilder> { | ||
| let mut wasi_ctx = WasiCtxBuilder::new(); | ||
| wasi_ctx.allow_tcp(true).allow_udp(true); |
There was a problem hiding this comment.
One thing on this -- this probably only wants to happen if sockets/addresses are otherwise allowed for this component. Otherwise this is technically a vector by which guests can create fds on the host and may cause minor issues. Would it be possible to make these conditional based on other settings? Or are guests expected to always be able to create a socket and outgoing connections?
There was a problem hiding this comment.
Should be possible to extend OutboundAllowedHosts with a "is this protocol ever allowed" method.
| let trap = err | ||
| .root_cause() // The error returned is a backtrace. We need the root cause. | ||
| .downcast_ref::<I32Exit>() | ||
| .expect("trap error was not an I32Exit"); | ||
| assert_eq!(trap.0, 1); | ||
| assert!(format!("{err:?}").contains("command failed")) |
There was a problem hiding this comment.
This looks a bit suspect to me, is I32Exit no longer showing up? If so that might be a bug
There was a problem hiding this comment.
I've gone back as far as Spin v3.6.0 and Rust 1.90 (and various versions in between), and this test is failing in that version and every version since (and possibly earlier). I have no idea how CI has been green all that time.
This came up during the Wasmtime 46 upgrade as well, but I wrote it off because it only seemed to be happening on my machine and not on CI. Now it's happening in both places, and I can't tell what changed.
There was a problem hiding this comment.
Ah ok if this is just a historical thing then that's fine yeah, I mostly just wasn't aware of anything in 48.0.0 that would have affected this
There was a problem hiding this comment.
I think this could be related to the version of Rust used to build the Wasm code (and consquently the version of WASI-SDK used?) which could influence whether an error exit means returning an err from wasi:cli/run vs. calling wasi:cli/exit{_with_code}, but I haven't had a chance to confirm.
There was a problem hiding this comment.
If the above is correct, then probably the best thing to do here is look for an I32Exit or a "command failed", since either could be valid depending on how the component was built.
There was a problem hiding this comment.
Ah yeah I wouldn't worry much about this, it's definitely unrelated to the 47->48 update which is all I wanted to bottom out
Bumps Wasmtime to v48.0.0 prerelease