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

AN-8945: Call reset() to delete ByteStream in ResourceStream #314

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

Conversation

ZakHsieh
Copy link

@ZakHsieh ZakHsieh commented Jun 6, 2018

Original implementation will cause huge memory leakages when Java layer acquire a resource, the ByteStream is never been released due to calling release() won't delete the memory allocation.
I'd change the _ptr.release() to _ptr.reset() to unlink the pointer and release the memory of ByteStream.

@danielweck
Copy link
Member

Although I have not tested this code change myself, I provided technical details in this reply to the original Pull Request:
#307 (comment)
As you can see, my preferred solution is the same as the one @ZakHsieh proposes in this Pull Request, so I personally approve this PR in principle (again, untested code at my end, but high degree of confidence that this works, especially as this is in production at @ZakHsieh 's company).
@rkwright feel free to merge if you consider this a consensus :)

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.

None yet

2 participants