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

Read memory from import #1724

Closed
Yamakaky opened this issue Oct 15, 2020 · 2 comments
Closed

Read memory from import #1724

Yamakaky opened this issue Oct 15, 2020 · 2 comments
Labels
🎉 enhancement New feature!

Comments

@Yamakaky
Copy link

Motivation

I'm writing an error handler which converts wasm panics into wasmer RuntimeError using RuntimeError::raise. I do that by calling raise from a custom_exit import. This import is called in wasm using std::panic::set_hook. The panic callback convert the PanicInfo into a string, then calls custom_exit with the str pointer and len.

One trouble I have is about getting a handle to Memory from the callback rust-side. It needs a handle to the instance memory for new_native_with_env to create the imports, but this memory is only accessible after creating the instance. I ended up using a Rc<RefCell<Option<Memory>>> as env and init it after instance creation, but it looks horrible.

Proposed solution

There should be a way to access the instance memory from the instance. In a lot of case, native wasm types will not be enough and you will need read and write the Wasm memory. Not sure how that would work out, though... Maybe by passing as first argument?

Alternatives

Additional context

@Yamakaky Yamakaky added the 🎉 enhancement New feature! label Oct 15, 2020
@MarkMcCaskey
Copy link
Contributor

Thanks for the feedback! We agree that the current API is not where we'd like it to be.

The implementation that we're currently using internally for WASI is quite complicated but you can take a look at it here https://github.com/wasmerio/wasmer/blob/master/lib/wasi/src/lib.rs#L52-L55 .

We are actively discussing a number of proposals for API changes to make this ergonomic! I think this is the most important thing we need to address in the Wasmer API right now so I'm trying to prioritize it, but it's a potentially big change and something that will have implications for our API for a long time so I want to make sure we find a scalable solution that we're happy with.

I'll try to remember to post about it publicly when we have more news to share here!

@Hywan
Copy link
Contributor

Hywan commented Jul 16, 2021

Since Wasmer 2.0, there is no more panic. All “panics” are now translated to RuntimeError. I am likely to be wrong but I think it's been solved in #2305.

@Hywan Hywan closed this as completed Jul 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature!
Projects
None yet
Development

No branches or pull requests

3 participants