Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Make R reset the cart in native runtime #530

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

desttinghim
Copy link
Contributor

Closes #503

@aduros
Copy link
Owner

aduros commented Sep 6, 2022

I think this has a memory leak, w4_runtimeInit and w4_wasmLoadModule aren't currently designed to be called multiple times.

@desttinghim
Copy link
Contributor Author

Oops, I didn't think to check for that. I'll investigate and fix it

@desttinghim
Copy link
Contributor Author

I ran valgrind on this branch and on main, and I don't see any difference in the output. I think the existing memory is being re-used, and nothing is being allocated on reset. Let me know if I'm missing something obvious

@aduros
Copy link
Owner

aduros commented Sep 12, 2022

It's possible that the memory is reused, but that might be undefined behavior. Also, having an explicit teardown method would make it easier to safely port to other wasm engines.

@desttinghim
Copy link
Contributor Author

desttinghim commented Oct 5, 2022

w4_runtimeInit looks like it can be called again no problem - it clears all the memory to zero and resets everything to default. w4_wasmLoadModule sets up the actual runtime, which doesn't need to be repeated, and probably shouldn't be either. The memory just needs to zeroed, and then the start function needs to be called if I understand correctly.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Native Runtime: Add R key to restart cart
2 participants