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

Add test that shows local memory/table lifetime bug #2131

Closed
wants to merge 1 commit into from

Conversation

MarkMcCaskey
Copy link
Contributor

The issue is that we expect these things to work while the store/engine is alive but critical data is stored in the Instance itself and the Instance does not live as long as the engine/store.

Review

  • Add a short description of the the change to the CHANGELOG.md file

@MarkMcCaskey MarkMcCaskey requested a review from Hywan February 18, 2021 16:13
@syrusakbary syrusakbary mentioned this pull request May 1, 2021
5 tasks
bors bot added a commit that referenced this pull request May 3, 2021
2281: Refactor vm r=syrusakbary a=syrusakbary

<!-- 
Prior to submitting a PR, review the CONTRIBUTING.md document for recommendations on how to test:
https://github.com/wasmerio/wasmer/blob/master/CONTRIBUTING.md#pull-requests

-->

# Description

This PR simplifies the VM code. and improves it's resiliency:
* [x] Removed unnecessary Exported structs
* [x] Reuse same VM data when available. Fixing #2131 
* [x] Removed unnecessary Function Definition in the API 
* [ ] Fixes the NativeEngine by automatically deallocating the libraries when the module is no longer used

<!-- 
Provide details regarding the change including motivation,
links to related issues, and the context of the PR.
-->

# Review

- [ ] Add a short description of the change to the CHANGELOG.md file


Co-authored-by: Syrus <[email protected]>
Co-authored-by: Syrus Akbary <[email protected]>
@Hywan
Copy link
Contributor

Hywan commented Jul 19, 2021

@syrusakbary Is it closed by #2281?

@Hywan Hywan added 🧪 tests I love tests 🏖️ sandbox A bug related to sandboxing bug Something isn't working and removed 🏖️ sandbox A bug related to sandboxing labels Jul 19, 2021
@syrusakbary syrusakbary removed the request for review from Hywan October 28, 2021 15:42
@syrusakbary syrusakbary assigned Amanieu and unassigned syrusakbary Oct 28, 2021
@syrusakbary
Copy link
Member

We need to check if this issue still exists on master @Amanieu

@syrusakbary syrusakbary added the 🕵️ needs investigation The issue/PR needs further investigation label Oct 28, 2021
@epilys
Copy link
Contributor

epilys commented Jul 1, 2022

No longer relevant since objects are now owned by the Context/Store and the actual Memory, Global etc are id handles to the allocated objects (see #2892)

@epilys epilys closed this Jul 1, 2022
@epilys epilys deleted the fix/local-memory-lifetimes branch July 1, 2022 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 🕵️ needs investigation The issue/PR needs further investigation 🧪 tests I love tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants