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

Strings not freed in the JS wrapper? #236

Closed
andrewwhitehead opened this issue Aug 18, 2023 · 3 comments
Closed

Strings not freed in the JS wrapper? #236

andrewwhitehead opened this issue Aug 18, 2023 · 3 comments

Comments

@andrewwhitehead
Copy link
Member

I don't see any references to anoncreds_string_free, but this is likely required for all strings coming from the FFI layer. This could be a significant memory leak over time.

@berendsliedrecht
Copy link
Contributor

Yeah that does not seem to be too good. I think we have three solutions but they all have some disadvantages

  1. Expose string_free publicly and allow the consumer to deal with it.
  • Downside is that this is quite the uncommon pattern in JS and likely they will still forget some allocations.
  1. Within the wrapper copy the value and immediately call it (it might even already be copied ffi-napi and c++ side).
  • Would still require all manual frees but at least we control it and not the consumer.
  1. Wait for Typescript 5.2 to be stable as it introduces the using keyword which would finally allow a cleanup function to be ran when it goes out-of-scope.

IMO it is okay to wait for typeScript 5.2 and leave this as-is for now.

@genaris
Copy link
Contributor

genaris commented Aug 18, 2023

3. Wait for Typescript 5.2 to be stable as it introduces the using keyword which would finally allow a cleanup function to be ran when it goes out-of-scope.

Well, if that iteration plan is accurate, it will be out in 4 days from now! So definitely it's worth waiting a bit and doing not only this but also for object handle clear() methods.

If not, I'd go for option 2.

@andrewwhitehead
Copy link
Member Author

I think the wrapper copies these FFI string buffers to Strings anyway, so it should be possible to free the reference before returning. I noticed later that this would apply to anoncreds_buffer_free as well, for binary buffers.

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

No branches or pull requests

3 participants