vmctx leaks? #1584
Comments
bors bot
added a commit
that referenced
this issue
Dec 15, 2020
1865: Fix memory leak in host function envs r=MarkMcCaskey a=MarkMcCaskey TODO: link to issue This PR contains a number of changes: 1. Make `WasmerEnv: Clone` 2. Store a pointer to the `clone` function when creating a host function (Notably this is a feature that wouldn't work even if we _could_ use a proper trait object because you can't have a `Sized` trait object and `Clone: Sized`). 3. Store a pointer to the `drop` function when creating a host function. 4. Clone the env via pointer every time an `Instance` is made. Therefore each `Instance` gets its own, unique `Env` per host function with `Env`. 5. Add reference counting and drop logic to a sub-field of `wasmer_export::ExportFunction` which frees the original version of the `Env` (the thing that gets cloned each time an `Instance` is made) with the `drop` function pointer. 6. Change some logic in `vm::Instance` from SoA (struct of arrays) to AoS (array of structs): this uses more memory but is a bit less error prone and can be easily changed later. 7. Add logic on this new struct (`vm::ImportEnv`) that contains the function pointers for each import in `Instance` to drop (with the `drop` fn pointer) when the `vm::Instance` is being dropped. This fixes the original memory leak. 8. Add wrapper functions inside the host function creation functions which makes the layout of the user supplied env-pointer the responsibility of each function. Thus, rather than `drop` being `Env::drop`, it's a function which frees all wrapper types, traverses indirections and frees the internal `Env` with `Env::drop`. This simplifies code at the cost of making the `host_env` pointer (`vmctx`) not consistent in terms of what it actually points to. This change fixes another memory leak related to the creation of host functions. tl;dr: we're leaning into manually doing virtual method dispatch on `WasmerEnv`s and it actually works great! The biggest issue I have with the PR as-is is that the code isn't as clean/readable/robust as I'd ideally like it to be. Edit (by @Hywan): This PR fixes #1584, #1714, #1865, #1667. # Review - [ ] Add a short description of the the change to the CHANGELOG.md file Co-authored-by: Mark McCaskey <mark@wasmer.io> Co-authored-by: Mark McCaskey <5770194+MarkMcCaskey@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It seems that
vmctxis memory-leaked here:Function::new_with_envwasmer/lib/api/src/externals/function.rs
Line 137 in 518e0f3
Function::new_native_with_envwasmer/lib/api/src/externals/function.rs
Line 230 in 518e0f3
The text was updated successfully, but these errors were encountered: