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 #10

Open
julioyg opened this issue May 9, 2016 · 6 comments
Open

Memory Leak #10

julioyg opened this issue May 9, 2016 · 6 comments

Comments

@julioyg
Copy link

julioyg commented May 9, 2016

Using you library in our project (thanks a lot for it) in which we use CanaryLeak I've found two leaks in your code:

  1. There are some anonymous AnimatorUpdateListener defined that holds a reference to the context.
  2. There are some anonymous TypeEvaluator implementations that because they holds references to their parent class and this one to the context leak memory too.
@julioyg julioyg mentioned this issue May 9, 2016
@julioyg
Copy link
Author

julioyg commented May 9, 2016

PR: #11

@tajchert
Copy link
Owner

tajchert commented May 9, 2016

How about PR: #6 ? Isn't it better?
I didn't have enough time to check all commits related to #6

@julioyg
Copy link
Author

julioyg commented May 9, 2016

looks great! you have any forecast or should we just download the code and work with it instead using the one in jcenter?

@tajchert
Copy link
Owner

tajchert commented May 9, 2016

I just checked as it works fine, didn't have time to find leaks and verify that those commits indeed solves particular leaks. If you can you can test it for leaks you have found, and ping me here with verdict if it works fine - I will merge it at that point if everything is fine. Just need somebody else to confirm it :)

@julioyg
Copy link
Author

julioyg commented May 10, 2016

checked #6:, leaks memory too because of the AnimatorUpdateListeners on the animators, the are never being removed

This is the screenshot I get from LeakCanary(which usually helps me more than the log)

image

@tajchert
Copy link
Owner

I've merged your #11 PR, as #6 is still getting some leaks and (sorry to say) I currently don't have time to go through it - anybody willing do fix other leaks (if there are any) are more than welcome.

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

No branches or pull requests

2 participants