Skip to content

Is this a leak? #505

Closed
Closed
@bogardon

Description

@bogardon
[[[@[@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...

Activity

jspahrsummers

jspahrsummers commented on May 21, 2013

@jspahrsummers
Member

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

bogardon

bogardon commented on May 21, 2013

@bogardon
Author

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

jspahrsummers

jspahrsummers commented on May 21, 2013

@jspahrsummers
Member

Would you mind uploading a saved Instruments trace somewhere?

bogardon

bogardon commented on May 21, 2013

@bogardon
Author
bogardon

bogardon commented on May 21, 2013

@bogardon
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

jspahrsummers commented on May 22, 2013

@jspahrsummers
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

bogardon commented on May 22, 2013

@bogardon
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

jspahrsummers commented on May 22, 2013

@jspahrsummers
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

bogardon commented on May 22, 2013

@bogardon
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

kkazuo commented on May 22, 2013

@kkazuo
Contributor

@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

Coneko commented on May 22, 2013

@Coneko
Member

@kkazuo's change does fix the problem.

Could you open a PR with the fix please?

added a commit that references this issue on May 22, 2013
6dd8007

2 remaining items

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @kkazuo@bogardon@jspahrsummers@Coneko

    Issue actions

      Is this a leak? · Issue #505 · ReactiveCocoa/ReactiveCocoa