[water-api] new API design draft #2
Conversation
Revised WATER design with new C-API from refraction-networking/wasmtime.
|
For code-review purposes, you might want to start from |
core.go
Outdated
|
|
||
| // get version | ||
| // In a _version() call, the WASM module will return its version | ||
| ret, err := c._version.Call(c.store) |
There was a problem hiding this comment.
Not too sure how Go handles function with no return, but I forgot to add a check for it if there is no return value for the _version function in Rust previously.
There was a problem hiding this comment.
If the function returns successfully, then the
interface{}return argument will be the result of the function. If there were 0 results then this value isnil. If there was one result then this is that result. Otherwise if there were multiple results then[]Valis returned.
From func (*Func) Call()'s definition.
There was a problem hiding this comment.
But let's forget about the multi-value for V0 (version in terms of of water API). It is not FFI-friendly and we don't take risks.
There was a problem hiding this comment.
It's way neater in Go, not like Rust, you need to pass in a &mut [Val] with the exact length of the return values, otherwise it will fail the call.
There was a problem hiding this comment.
fail the call
What's the behavior then? Returning some bad values, an error (a trap), or panic the entire host program (that's the worst)?
There was a problem hiding this comment.
Yeah, it will panic the the entire host 🫠, it's returning a Result<Err(E)> if I do .unwrap() or ? then it will panic the host where it's actually avoidable by adding some handling code for that case. But currently I'm passing something like this to the call to avoid it.
let mut res = vec![Val::I32(0); version_fn.ty(&self.store).results().len()];
runtime_conn_v0.go
Outdated
| } | ||
|
|
||
| // push WAConfig | ||
| configFd, err := rcv.core.store.PushFile(rcv.core.config.WAConfig.File(), wasmtime.READ_ONLY) |
There was a problem hiding this comment.
It might be better to ask WASM for the fd? Since it might cause conflict when there are many files that is open in WASM but closed in Host + many fd's for network streams. The way I was doing is fix config file's fd to be always 3 since in the pipeline, and we can have a global_available_fd in WASM to keep track on the next available fd, so there won't be any conflict.
There was a problem hiding this comment.
I thought that's what PushFile does:
PushFile keeps track of next unused fd in WasiCtx and binding the file to it. That's what makes it different from InsertFile.
Am I getting it wrong or something?
There was a problem hiding this comment.
My bad, I forgot that you were using PushFile, I should too!
Now example is working.
File descriptors will not work on Windows anyways...
Add (*Config).WASIConfig() to either return the already set WASIConfigFactory or create a new one.
Note: Corresponding sample
.wasmfile, tests, documentations and README still pending.RuntimeConninto aninterface{}/net.Conn, support future multi-versioning.WAConfiginto WASM.RuntimeListenerto directly listen on a local TCP port or Unix address