Navigation Menu

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

Is this a leak? #505

Closed
bogardon opened this issue May 21, 2013 · 12 comments · Fixed by #506
Closed

Is this a leak? #505

bogardon opened this issue May 21, 2013 · 12 comments · Fixed by #506
Assignees
Labels

Comments

@bogardon
Copy link

[[[@[@1,@2,@3,@4,@5] rac_sequence] filter:^BOOL(id value) {
    return [value intValue] > 1;
}] array];

I ran this in instruments and:

screen shot 2013-05-21 at 2 32 55 pm

Just thought I put this here while I try to figure this out...

@jspahrsummers
Copy link
Member

Is that the exact code you used? Can you share the context for it?

@bogardon
Copy link
Author

@jspahrsummers that's the exact code. I created an empty project and placed those lines at the top of appDidFinishLaunching....

@jspahrsummers
Copy link
Member

Would you mind uploading a saved Instruments trace somewhere?

@bogardon
Copy link
Author

@bogardon
Copy link
Author

It looks like the leak is happening because of this extra retain highlighted here.

screen shot 2013-05-21 at 2 56 38 pm

@jspahrsummers
Copy link
Member

I tried writing a unit test to reproduce this issue, but wasn't able to trigger a leak: https://github.com/ReactiveCocoa/ReactiveCocoa/compare/sequence-leak

@bogardon
Copy link
Author

How come you're not going through [RACDynamicSequence sequenceWithLazyDependency... in these tests?

Were you able to reproduce my results in instruments at all? FWIW it appears to be the same on the device.

Also, turns out it leaks even if you don't convert back to an array, but I guess your tests show that you already know that.

@jspahrsummers
Copy link
Member

@bogardon -bind: will invoke that method, and should touch all the same code paths that your -filter: example did.

I actually do see something similar when profiling our app, but nothing that helps me figure out the exact leak, why it's occurring, or how to best fix it. I was hoping a unit test could narrow it down and give us something reproducible. :\

@bogardon
Copy link
Author

If I change line 103/104 in RACSequence.m to have __weak modifiers in front. Instruments claims RACArraySequence is no longer leaked, but a few malloc 32 bytes are still leaking. Sorry at this point I'm just trying to brute force my way to a fix.

What's really confusing me now is why some of the items that Instruments claims to have leaked shows an allocation history where it reaches a reference count of 0, like this:

screen shot 2013-05-22 at 2 15 03 am

I googled around and people say it's related to not calling [super dealloc], which should be irrelevant for us since we're using ARC?

@kkazuo
Copy link
Contributor

kkazuo commented May 22, 2013

@bogardon How happen with this patch?

diff --git a/ReactiveCocoaFramework/ReactiveCocoa/RACDynamicSequence.m b/ReactiveCocoaFramework/ReactiveCocoa/RACDyn
index bd2eb18..0d935a6 100644
--- a/ReactiveCocoaFramework/ReactiveCocoa/RACDynamicSequence.m
+++ b/ReactiveCocoaFramework/ReactiveCocoa/RACDynamicSequence.m
@@ -97,9 +97,9 @@
        NSCParameterAssert(headBlock != nil);

        RACDynamicSequence *seq = [[RACDynamicSequence alloc] init];
-       seq.headBlock = headBlock;
-       seq.tailBlock = tailBlock;
-       seq.dependencyBlock = dependencyBlock;
+       seq.headBlock = [headBlock copy];
+       seq.tailBlock = [tailBlock copy];
+       seq.dependencyBlock = [dependencyBlock copy];
        seq.hasDependency = YES;
        return seq;
 }

It is dangerous with my experience that use of @property, (copy), Block type variable combination.
There is probably a some difference with @property(copy) void(^block)(); self.block = blockValue; and @property(strong) void(^block)(); self.block = [blockValue copy];.
May be compiler bug?
I always use @property(strong) id block and typecast for inoke block in my project code.

@Coneko
Copy link
Member

Coneko commented May 22, 2013

@kkazuo's change does fix the problem.

Could you open a PR with the fix please?

@jspahrsummers
Copy link
Member

That fix was truly bizarre, but I've tagged #506 in a v1.8.1 tag.

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

Successfully merging a pull request may close this issue.

4 participants