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 release callback #91

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add release callback #91

wants to merge 2 commits into from

Conversation

kylebarron
Copy link
Owner

@kylebarron kylebarron commented Jan 31, 2024

It's pretty astounding that this appears to work! Just the fact that it doesn't error is a good sign!

There's a lot of thought still in the memory management between Wasm and JS. We can't call the release callback if we're creating views, because the lifetime of the view might outlive the lifetime of the data. Even if we're copying data to JS, the user might want to do something with their wasm table after viewing it in JS.

So it might be best to keep the release functions standalone. That said, freeing the underlying array will not free the wasm-bindgen wrapper, and if we freed the wrapper after, that would lead to a double-free.

Closes #14

@kylebarron
Copy link
Owner Author

We also need to figure out some way to test that this is actually calling the release callback and that the referenced memory is actually deallocated.

@kylebarron
Copy link
Owner Author

You should be able to check that the release callback is now null https://arrow.apache.org/docs/format/CDataInterface.html#released-structure

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 this pull request may close these issues.

C Function pointers
1 participant