Allow setting fs in WASI config
#303
Conversation
Using some boilerplate to get the instance from a JS object.
|
Hey, thanks for the PR @corwin-of-amber . Could you also add tests to verify that the API works as expected? Thanks! |
|
Did it :) |
|
I think this fix has a bug: it seems that Rust takes ownership of the passed object making it invalid to use it from JS. This should probably not be the correct behavior. I am testing a modified version now. |
The JS instance becomes unusable, and trying to access it from JS leads to read from freed memory. Since MemFS is garbage-collected anyway (via Arc) it makes sense to clone the reference. JS clients should explicitly free if they do not want to use the fs instance anymore, to have the reference count decremented.
corwin-of-amber
force-pushed
the
fs-setting
branch
from
0ffce9d
to
8278302
Compare
|
I don't really know how to test for the buggy case, because the test passed even before the fix. Thing is, even if a dangling pointer exists the accesses will still succeed immediately after the call, because the memory has not been reclaimed/overwritten yet. This is what made it hard to see the problem in the first place. |
|
wdyk I found the exact same function in Wasmer API: probably from the same source. |
|
Were you able to move forward with this @corwin-of-amber ? |
|
I do not have a systematic approach to testing but I have used this branch for quite some time and tried different programs including ocamlrun and busybox, so I am pretty confident that it does not crash. |
If you are wondering about this, yes I assume wasmer-js can use the function from js/trap, but I do feel that my version provides better error reporting so perhaps you should merge them? (actually I have improved the error reporting further in my other branch so I should update this PR) |
Used e.g. when passing MemFS via config.
I noticed the use case where
fsis passed in config is unhandled, and I assume it was because wasm-boundMemFSdoes not implementJsCast, making it hard to pass it to JS and then get it back in the WASI ctor.I have adopted some common wisdom that I've found on github to get the wrapped instance from a JS object and pass it along. Are you interested?