CODEWITHSUNDEEP
objective-ccocoaconcurrencynsarrayobjective-c-blocks
Pripyat
Pripyat

Reputation: 2937

enumerateObjectsWithOptions:usingBlock: causing frequent unexplained freezes

I'm trying to enumerate through arrays using the enumerateObjectsWithOptions:usingBlock method. However, my code rarely works. When it does not work, my app freezes (but no beach ball) -- I'm relatively new to blocks, so I must be missing something that is required for block operations to work smoothly.

Note: I know I can enumerate using for loops, but this is not what I am looking for.

The troublesome code:

   NSArray*_storageCookies = [[NSArray alloc] initWithArray:[storage cookies]];

    NSArray*_historyObjects = [[NSArray alloc] initWithArray:[_history webkitHistory]];

    NSOperationQueue*_queue = [[NSOperationQueue alloc] init];

    NSBlockOperation*_block = [NSBlockOperation blockOperationWithBlock:^{

        NSAutoreleasePool*_pool = [[NSAutoreleasePool alloc] init];

        [_storageCookies enumerateObjectsWithOptions:NSEnumerationConcurrent usingBlock:^(id ck, NSUInteger index, BOOL *stop) 
        {            
            NSString*domain = [ck domain];

            [_historyObjects enumerateObjectsWithOptions:NSEnumerationConcurrent usingBlock:^(id object, NSUInteger aindex, BOOL *stop) 
            {
               @synchronized(self)
               {
                NSAutoreleasePool*_pool2 = [[NSAutoreleasePool alloc] init];

                NSString*_historyURL = [[NSURL URLWithString:[object url]] host];

                if ([_historyURL rangeOfString:domain].location != NSNotFound)
                {
                    NSHTTPCookie*cookie = [DAHTTPCookie createCookieWithURL:[ck domain] cookieName:[ck name] expires:[[ck expiresDate] timeIntervalSince1970] cookieValue:[ck value] browserType:DAWebkitBrowser secure:[ck isSecure]];

                    if ([_cookies containsObject:cookie] == NO)
                    {
                        [_cookies addObject:cookie];
                    }
                }

                [_pool2 release];
}
             }];
        }];

        [_pool release];
    }];

    [_queue addOperations:[NSArray arrayWithObject:_block] waitUntilFinished:YES];

    NSLog(@"Done - found %i Cookies...",[_cookies count]); //sometimes returns 0, sometimes the right number or nothing

Edit

I have fixed it. You were right, my class was not thread safe. So I had to add an @synchronized block for this to work as expected.

Upvotes: 1

Views: 1989

Answers (2)

Firoze Lafeer
Firoze Lafeer

Reputation: 17143

I don't see where _cookies is defined, but I guess we can assume it is an NSMutableArray. Those are not threadsafe, so if you have multiple threads reading and also adding objects to that array at the same time, that is a problem.

There are different ways to solve that. One option is to create a serial queue that owns that array. In the course of your processing, each time you need to check for (and if needed add) a cookie, schedule a small block on that queue to do that work. That way you can ensure those reads and adds are strictly serial.

There are other ideas for this also of course.

But then taking a step back, once you introduce that dance to keep the threads from stepping all over those _cookies, you may or may not find this is the most efficient implementation. You'll need to test that with your actual data.

Also, why the NSOperation here? Since you wait for the results anyway, why not just kick off the enumeration on the main queue and then let it fan out from there? (I may be missing something)

Upvotes: 2

bbum
bbum

Reputation: 162722

When you say "I'm using for loops and it works", do you mean that your for loops are running concurrently or that they are, effectively, running linearly?

Looking at that code, there is nothing wrong with the use of blocks. While it is impossible to conclusively say without full knowledge of the APIs you are using, the underlying algorithm in that code is effectively explosively parallel.

That is, each of your loops is configured to try and process stuff concurrently as much as possible. Unless every bit of the API you are calling is thread safe, then your code is likely crashing and/or locking up (the randomness of the symptoms are a sure sign of concurrency issues) because of multi-threaded conflicts.

Even if all the APIs are thread safe, explosive concurrency is pretty much never the right concurrency model. At best, you'll want throttled concurrency; you want to use some kind of a mechanism to limit the amount of junk being processed concurrently.

Upvotes: 4

Related Questions