Skip to content
This repository has been archived by the owner on Aug 24, 2019. It is now read-only.

Fetch bleeds significant memory when called in a for loop. #116

Closed
tylercrouch opened this issue Oct 21, 2015 · 18 comments
Closed

Fetch bleeds significant memory when called in a for loop. #116

tylercrouch opened this issue Oct 21, 2015 · 18 comments

Comments

@tylercrouch
Copy link

I tried calling the fetch command every .5 seconds to get one item (string with 4 characters) to test for memory bleeds. It starts to seriously eat at my applications memory and I have no idea why. My test code looks like this:

-(void) infiniteLoop{

NSError *error = nil;
SSKeychainQuery * query = [[SSKeychainQuery alloc] init];
query.service = @"test";
query.account = @"loginPINForAccountID-2";
query.password = nil;
[query fetch:&error];

@autoreleasepool {
    query = nil;
    error = nil;
}

[self performSelector:@selector(infiniteLoop) withObject:nil afterDelay:.5f];

}

Any advice here would be greatly appreciated.

@tylercrouch tylercrouch changed the title Fetch bleeds memory like crazy when called in a for loop. Fetch bleeds significant memory when called in a for loop. Oct 21, 2015
@calebd
Copy link
Collaborator

calebd commented Oct 21, 2015

Infinite recursion is almost certainly not what you want. There is probably a much better way to do this.

@tylercrouch
Copy link
Author

Oh yes I am sure! I just wanted to make sure that the code won't bleed if I call it many times over a use session. Is there a better way to test that? I think a user may call this over 100 times in 24 hours in maximum use case. I have a lot to learn with objc still and could use any input you could give me here :)

@calebd
Copy link
Collaborator

calebd commented Oct 21, 2015

It absolutely will. You are infinitely allocating memory. What are you trying to accomplish?

@tylercrouch
Copy link
Author

I am trying to understand how to fetch data using SSKeychain in a way that will not bleed memory. I want to fetch data, then dispose of the resources used to fetch the data as well as the data itself to end with a net memory allocation of zero. The idea here is that I want to keep some confidential credentials in the keychain and only pull them out to check them against user input or use for a given API call. This would require me to call the SSKeychain fetch many many times in a use session hence me trying to test to see if it will bleed.

Is there a way to do this? Am I being naive here?

@tylercrouch
Copy link
Author

Also I assumed my autorelease would deallocate the memory I allocated for the SSKeychainQuery.

@calebd
Copy link
Collaborator

calebd commented Oct 21, 2015

Then only pull the keychain items when you actually need them. Memory is only released when the release pool is drained at the end of a run loop. That will never happen if you infinitely recurse. And your custom release pool doesn't do anything because you aren't allocating any memory inside it.

@calebd
Copy link
Collaborator

calebd commented Oct 21, 2015

This is not a bug in SSKeychain.

@calebd calebd closed this as completed Oct 21, 2015
@tylercrouch
Copy link
Author

Sorry to ping this as a bug, I am new to github - this was definitely more of a question on a use case.
Even if I modify my code to call the following loop only once I still get a bleed:

for (int i = 0; i < 10000; ++i) {
        @autoreleasepool {
            NSError *  error2 = nil;
            SSKeychainQuery*  query2 = [[SSKeychainQuery alloc] init];
            query2.service = @"Eko";
            query2.account = @"loginPINForAccountID-2";
            query2.password = nil;
            [query2 fetch:&error2];
            query2 = nil;
        }
}

I have been searching online for a while now, can you help this lost soul out?
Point me in the right direction? I am really worried that this will cause significant drain in my app over long use sessions and I don't know where to look for a solution. You mentioned run loops and I read through the documentation but that isn't really helping me.

@calebd
Copy link
Collaborator

calebd commented Oct 21, 2015

I still don't understand what you are trying to do, or why polling the keychain multiple times a second would be useful.

The objects you are creating are not eligible for autorelease. alloc] init] returns a retained object. Internally, fetch: uses SecItemCopyMatching which also returns a retained object. ARC will wait until a convenient time before it attempts to clean memory that no longer has any strong references. You are likely preventing that from happening by looping like that.

@tylercrouch
Copy link
Author

I am trying to make a test to simulate 24 hours of my applications use where fetch would get called many times at irregular intervals. I don't want to sit around for a whole day so I thought this loop would simulate the extreme use situation. I tried having the loop wait 10 seconds or so between polls but the result still seems to be the same (large memory overhead). If ARC cleans up my allocations then wouldn't it get deallocated after the loop ends and time passes? I waited 20 minutes after the loop ended and the memory still remained. I feel like I must be doing something fundamentally wrong at this point or that this test is really unrealistic.

@calebd
Copy link
Collaborator

calebd commented Oct 21, 2015

Have you tried running the leaks instrument?

@tylercrouch
Copy link
Author

Yeah that's where I started. The loop I made is basic and the only thing in my entire test project so it's no surprise that the leak points to SecItemCopyMatching. Anyways I gather from this conversation that one is only supposed to use the load command in your code very sparingly and optimally once per unique object per app use session. I was hoping to ping your keychain code once every minute or so but I guess that is a very bad idea. Perhaps I shouldn't be using keychain at all and just encrypt the password/username data myself and throw it into core data to avoid the memory drain over time.

@tylercrouch
Copy link
Author

Anyways, thanks for the help. I am sure there must be some way to use your code and not create leaks from continuous use but I am not seeing it. :/

Back to the drawing board!

@calebd
Copy link
Collaborator

calebd commented Oct 21, 2015

If you found a leak I'd be happy to apply a fix or accept a pull request. fetch: uses a __bridge_transfer cast to bring the +1 object into ARC which should release once it is no longer needed.

@tylercrouch
Copy link
Author

How can I make it known that it is no longer needed?

        NSError *  error2 = nil;
        SSKeychainQuery*  query2 = [[SSKeychainQuery alloc] init];
        query2.service = @"Eko";
        query2.account = @"loginPINForAccountID-2";
        query2.password = nil;
        [query2 fetch:&error2];
        query2 = nil;

Doesn't me setting query2 to nil here tell the code I don't need that anymore?

@calebd
Copy link
Collaborator

calebd commented Oct 21, 2015

You do not need to set it to nil. Letting it fall out of scope would be functionally equivalent. When the object is deallocated depends on how the ARC calls are inserted.

@tylercrouch
Copy link
Author

If my for loop ends and so does my method (which never gets called again) shouldn't the variables then fall out of scope?

@AnZhg
Copy link

AnZhg commented Dec 14, 2017

@tylercrouch It could also be that the Security.framework is leaking. When running leak tools profiling an app built against macOS 10.13 SDK, I've seen CFDictionaries leaked inside Security framework.

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

No branches or pull requests

3 participants