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

Maybe remove the not necessary runtime.KeepAlive() #147

Open
howjmay opened this issue Sep 8, 2022 · 1 comment
Open

Maybe remove the not necessary runtime.KeepAlive() #147

howjmay opened this issue Sep 8, 2022 · 1 comment

Comments

@howjmay
Copy link
Contributor

howjmay commented Sep 8, 2022

According to this discussion https://forum.golangbridge.org/t/curious-about-runtime-keepalive-usage/3530, some common scenarios that need to call runtime.KeepAlive() is

  1. you have obtained the file descriptor to an underlying os.File.
  2. you have passed memory to cgo which was allocated inside the same function.

In wasmtime-go, the variable that we are calling runtime.KeepAlive() for are normally not allocated by the same function, so I think maybe we don't need runtime.KeepAlive()?

@alexcrichton
Copy link
Member

Do you have some examples of invocations that can be removed? I tried to write things in such a way that removing any necessary call would trigger a panic, but it's also not guaranteed to panic so it's just a small safeguard.

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

Successfully merging a pull request may close this issue.

2 participants