SwiftKickMobile / TLLayoutTransitioning

Enhanced transitioning between UICollectionView layouts in iOS.
MIT License
355 stars 47 forks source link

Memory leak #6

Closed holidaycottages closed 10 years ago

holidaycottages commented 10 years ago

Hello, I am using your collectionview transition in my app and I appear to have a memory leak and xcodes profiler indicates it is here:

UICollectionViewTransitionLayout transitionLayout = [self startInteractiveTransitionToCollectionViewLayout:layout completion:^(BOOL completed, BOOL finish) { UICollectionViewTransitionLayout transitionLayout = [self tl_transitionLayout]; if ([transitionLayout conformsToProtocol:@protocol(TLTransitionAnimatorLayout)]) { idlayout = (id)transitionLayout; [layout collectionViewDidCompleteTransitioning:self completed:completed finish:finish]; } [self tl_setAnimationDuration:nil]; [self tl_setAnimationStartTime:nil]; [self tl_setTransitionLayout:nil]; [self tl_setEasingFunction:NULL]; if (completion) { completion(completed, finish); } [[UIApplication sharedApplication] endIgnoringInteractionEvents]; }];

I am unsure of how to fix this, can you help?

wtmoose commented 10 years ago

Hi holiday,

I checked both of the example projects and neither of them leak. But my guess is that your transition layouts are leaking. Can you confirm whether that is the case or not? If it is, then please make sure to use __weak as necessary to avoid retain cycles in your completion block, if you've got one. If that doesn't help, please send more info like your view controller's code, instruments output, etc.

holidaycottages commented 10 years ago

Hi wtmoose, thanks for replying. I could replicate a leak in both my project and the example project you supply, here is a screen grab of the examples project

https://www.dropbox.com/s/3quoq619oj9sbee/Screenshot%202014-03-25%2015.06.29.png

https://www.dropbox.com/s/ykncrrih290zf0m/Screenshot%202014-03-25%2015.09.32.png

What I did was profile it for a memory leak, select the resize option from the left and then just repeatedly pressed on a square, going back and forth, and then I get the above. Which leads to increased memory consumption until the ipad runs out!

Is that of any help?

holidaycottages commented 10 years ago

Xcode 5.1 running iOS7 base SDK too

wtmoose commented 10 years ago

Yep, you’re right. I was just looking at Allocations and didn’t see any noticeable growth.

I just promoted a fix. Please let me know if it works for you.

Tim

On Mar 25, 2014, at 10:08 AM, holidaycottages notifications@github.com wrote:

Hi wtmoose, thanks for replying. I could replicate a leak in both my project and the example project you supply, here is a screen grab of the examples project

https://www.dropbox.com/s/3quoq619oj9sbee/Screenshot%202014-03-25%2015.06.29.png

What I did was profile it for a memory leak, select the resize option from the left and then just repeatedly pressed on a square, going back and forth, and then I get the above. Which leads to increased memory consumption until the ipad runs out!

Is that of any help?

— Reply to this email directly or view it on GitHub.

holidaycottages commented 10 years ago

That fixed that leak, however it appears there is another here:

https://www.dropbox.com/s/t0r58az9vmv2y33/Screenshot%202014-03-25%2015.31.10.png

https://www.dropbox.com/s/m7oljk8a88lepng/Screenshot%202014-03-25%2015.32.14.png

:)

wtmoose commented 10 years ago

Can you provide steps for that one? I banged on both sample projects and didn’t see it. The only thing I found was a small leak in Apple’s code:

Also, how significant is this leak in your app?

Tim

On Mar 25, 2014, at 10:32 AM, holidaycottages notifications@github.com wrote:

That fixed that leak, however it appears there is another here:

https://www.dropbox.com/s/t0r58az9vmv2y33/Screenshot%202014-03-25%2015.31.10.png

https://www.dropbox.com/s/m7oljk8a88lepng/Screenshot%202014-03-25%2015.32.14.png

:)

— Reply to this email directly or view it on GitHub.

holidaycottages commented 10 years ago

I did exactly the same method as I did before to replicate the leak.

Each press of an item adds about 13mb of memory to the app, so only so many taps a user can do before it crashes. For me its quite important.

Is there anything else I can provide you with?

holidaycottages commented 10 years ago

Here are some more screen grabs if they help.

Just to tell you what i did, I amended the examples project with the changed file from the repo, and then ran it on an ipad, chose the resize option and then just clicked on a cell, then pressed again and repeated again and again in quick succession.

https://www.dropbox.com/s/k3t5c3knfd2pndd/Screenshot%202014-03-25%2015.59.32.png

https://www.dropbox.com/s/9jh4tmmbkdpsyjg/Screenshot%202014-03-25%2016.00.12.png

holidaycottages commented 10 years ago

Ah, it appears it could now be my code in the didSelectItemAtIndexPath......hmm

wtmoose commented 10 years ago

Ok, I’ll take another look if it turns back to TLLayoutTransitioning. The leaks in your screenshots look like they’re occurring in Apple's code. But I’m still unable to reproduce them in Simulator or on my iPad Air. I’m on Xcode 5.1 and iOS 7.1.

Tim

On Mar 25, 2014, at 11:15 AM, holidaycottages notifications@github.com wrote:

Ah, it appears it could now be my code in the didSelectItemAtIndexPath......hmm

— Reply to this email directly or view it on GitHub.

holidaycottages commented 10 years ago

No worries Tim, thank you so much for your quick response and fix!

It was my code, I wasnt removing MBProgressHUD properly!

wtmoose commented 10 years ago

No problem. Glad you got it fixed!

On Mar 25, 2014, at 12:28 PM, holidaycottages notifications@github.com wrote:

No worries Tim, thank you so much for your quick response and fix!

It was my code, I wasnt removing MBProgressHUD properly!

— Reply to this email directly or view it on GitHub.