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

Memory Leak #27

Open
mixerzeyu opened this issue Aug 15, 2020 · 4 comments
Open

Memory Leak #27

mixerzeyu opened this issue Aug 15, 2020 · 4 comments
Labels

Comments

@mixerzeyu
Copy link

mixerzeyu commented Aug 15, 2020

Hi - The circular slider is amazing!

I've noticed recently it does not cleanup properly and creates memory leaks. There seems to be a circular (no pun intended) reference somewhere within the controller, therefore it is not deallocated when the containing UIViewController exits.

You can see the memory leaks if you show memory graphs when running an Xcode project in debug mode, there would be multiple instances of the circular slider created after you open and close the UIViewController multiple times.

Let me know if you need more info.

Thanks!

@ThunderStruct
Copy link
Owner

Hi!

Thank you for shedding some light on this issue! I just profiled the example project on this repo and you're right, there seems to be a minor leak when the UIViewController is supposedly deallocated. I haven't analyzed it in depth yet, but it does seem to be a circular reference as you speculated!
I'll make sure to fix it in the next release once I get a chance.

Thanks again!

@Vasyltm
Copy link

Vasyltm commented Aug 28, 2020

Hi!

The problem is in MSCircularSlider:

func initHandle() {
handle.delegate = self
handle.center = {
return self.pointOnCircleAt(angle: self.angle)
}
}

self in closure creates retain cycle. We should use capture list here:

func initHandle() {
handle.delegate = self
handle.center = { [weak self] in
guard let self = self else { return CGPoint(x: 0, y: 0) }
return self.pointOnCircleAt(angle: self.angle)
}
}

@ThunderStruct
Copy link
Owner

Hey @Vasyltm

You're absolutely right! The self reference in the closure does in fact create a retain cycle. Thanks for the proposed solution as well. Will include it in the next update (and credit you in the changelog)!

@mixerzeyu
Copy link
Author

@Vasyltm Thanks so much for posting the solution - it works like a charm!
@ThunderStruct Would be great to know once this fix goes is updated :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants