search

readium / readium-sdk

A C++ ePub renderer SDK
BSD 3-Clause "New" or "Revised" License
388 stars 164 forks source link

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

Open ZakHsieh opened 6 years ago

ZakHsieh commented 6 years ago

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 commented 6 years ago

Although I have not tested this code change myself, I provided technical details in this reply to the original Pull Request: https://github.com/readium/readium-sdk/pull/307#issuecomment-364453156 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 :)