algolia / voice-overlay-ios

🗣 An overlay that gets your user’s voice permission and input as text in a customizable UI
https://alg.li/voice
MIT License
544 stars 62 forks source link

Replacing strong self with weak. #7

Closed rahimkhalid closed 5 years ago

rahimkhalid commented 5 years ago

updated the code to use weak self where previously using strong self.

spinach commented 5 years ago

Hello,

Thank you for your PR suggestion!

Your commit doesn't build, since when using [weak self], you need to us self? instead of self.

Also, I don't see what memory leaks it actually solves. Note that we are aware that when using the component, there are a few memory leaks that come from AudioToolBoox and Speech libraries, but these are Apple Frameworks, and it seems a bug from their side (check this and this. Let me know if that makes sense!

If you have a screenshot of the memory leak stack trace, please do include it in here.

rahimkhalid commented 5 years ago

Hi, I have updated the PR with your pointed issues, also I just tested the stuff and checked the links provided by you seems like memory leaks are in apple framework. But I think its better to use [weak self] in completion handlers. Also updated the PR description.

spinach commented 5 years ago

Hey @rahimkhalid ,

Sorry for the late reply and thanks again for your suggestion. After checking out the changes, and running instruments on Xcode to double check memory leaks, we see that they don't solve the intended problem. Doing weak self in this case is not necessary as there are no strong reference cycles. The few memory leaks that are occurring are due to Apple's framework and we can't control that.

Thanks again for the PR!