Using Singleton synchronized array with NSThread

Question

I have a books app with a UISearchBar, where the user types any book name and gets search results (from ext API call) below as he types.

I am using a singleton variable in my app called retrievedArray which stores all the books.

@interface Shared : NSObject {
    NSMutableArray *books;
}

@property (nonatomic, retain) NSMutableArray *books;

+ (id)sharedManager;

@end

This is accessed in multiple .m files using NSMutableArray *retrievedArray; ...in the header file

retrievedArray = [[Shared sharedManager] books];

My question is how do I ensure that the values inside retrievedArray remain synchronized across all the classes.

Actually the values inside retrievedArray gets added through an NSXMLParser (i.e. through external web service API). There is a separate XMLParser.m file, where I do all the parsing and fill the array. The parsing is done on a separate thread.

    - (void) run: (id) param  {
        NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];

        NSXMLParser *parser = [[NSXMLParser alloc] initWithContentsOfURL: [self URL]];
        [parser setDelegate: self];
    [parser parse];
        [parser release];

        NSString *tmpURLStr = [[self URL]absoluteString];

        NSRange range_srch_book = [tmpURLStr rangeOfString:@"v1/books"];

        if (range_srch_book.location != NSNotFound) 
            [delegate performSelectorOnMainThread:@selector(parseDidComplete_srch_book) withObject:nil waitUntilDone:YES];

        [pool release];
    } 


    - (void) parseXMLFile: (NSURL *) url
    {   
        NSAutoreleasePool *pool = [[NSAutoreleasePool alloc] init];
        [self setURL: url];
        NSThread* myThread = [[NSThread alloc] initWithTarget:self
                                                     selector:@selector(run:)


object: nil];
    [retrievedArray removeAllObjects];
    [myThread start];
    [pool release];
}

There seems to be some synchronization issues if the user types very quickly (It seems to be working fine if the user types slowly)....So there are 2 views in which the content of an object in this shared array item is displayed; List and Detail. If user types fast and clicks on A in List view, he is shown B in detail view...That is the main issue.

I have tried literally all the solutions I could think of, but am still unable to fix the issue.

EDITING FOR SYNC ISSUE EXAMPLE: In the list view, if there are 3 items shown, say Item1, Item2 and Item3 and if user clicks on Item2, he is shown Item3 in detail view (i.e. to say not the correct details)

Below is the code that gets executed when an item in list view is clicked;

- (void)tableView:(UITableView *)tableView didSelectRowAtIndexPath:(NSIndexPath *)indexPath {
    // Navigation logic -- create and push a new view controller

    if(bookdetailCustom == nil)
        bookdetailCustom = [[BookDetailCustom alloc] initWithNibName:@"BookDetailCustom" bundle:[NSBundle mainBundle]];

    //aBook = [retrievedArray objectAtIndex:indexPath.row];

    bookdetailCustom.selectedIndex = indexPath.row;

    [self.navigationController pushViewController:bookdetailCustom animated:YES];
    [bookdetailCustom release];
    bookdetailCustom = nil;
}

Here is how searchTabkleView looks like

- (void) searchTableView {
    NSString *searchText = searchBar.text;
    NSMutableArray *searchArray = [[NSMutableArray alloc] init];

    for (int i=0;i<[retrievedArray count];i++)
    {
        Stock *aBookTemp = [retrievedArray objectAtIndex:i];
        NSString *temp = [aBookTemp valueForKey:@"BookName"];
        [searchArray addObject:temp];
    }

    for (NSString *sTemp in searchArray)
    {
        NSRange titleResultsRange = [sTemp rangeOfString:searchText options:NSCaseInsensitiveSearch];

        if (titleResultsRange.length > 0)
            [copyListOfItems addObject:sTemp];
    }

    [searchArray release];
    searchArray = nil;
}

Please suggest some suitable fixes.

Answer 1

From what you've posted, every retrievedArray is pointing at the same NSMutableArray object. So there aren't any separate arrays to keep synchronized, it's all the same array.

However, NSMutableArray is not thread safe; things may blow up if one thread is changing it while another is reading it. Simply changing the property from nonatomic to atomic is insufficient, because that only covers fetching the array object itself and not the subsequent method calls to access elements inside the array. I don't think that is causing your main issue, though, and the fix for that should obviate the thread safety issue.

I guess the sequence of events is something like this:

1. The List view is displaying a set of results that includes A at index N.
2. The user types something. The XML parser starts to update the shared array incrementally. The List view isn't updated yet.
3. The user touches the item at index N in the List view. The list view instructs the Detail view to display the item at index N.
4. The Detail view extracts the item at index N from the shared array, but due to the update started in step 2 index N now contains B. Which the Detail view displays.
5. At some point the XML parse completes, and now List is updated.

It should also be possible, if the load and parse from the web service is slow enough, for step 4 to simply crash with an NSRangeException.

One solution would be for each item in the List to hold the actual result object and pass that to the Detail view rather than just the index. You might be able to completely get rid of the shared array in this case, if List and Detail are the only consumers or if any other consumers can be changed to take objects instead of indices in the same way. Another would be for the parser to accumulate the results into a private array, and update the shared array all at once just before signaling the List view to update itself; there is still a slight possibility for a race in the time between the update on the background thread and the method invocation on the main thread, but the window is probably quite a bit smaller.

Or I could be completely wrong in my guess on how the update works, in which case you should provide more details.

Answer 2

I originally suggested that you remove the nonatomic keyword from your property declaration. Atomic is the default (there is no atomic setting, omitting nonatomic is sufficient) - which will handle thread safety for you by wrapping the synthesized setter in a @synchronize block.

Unfortunately, many people have learned just to put nonatomic all over their code without really understanding it. I've always thought that this comes from copy/paste out of Apple sample code -- they use it often for UI-related stuff -- remember that UIKit isn't thread safe.

Anomie has indicated in his/her answer that this isn't it -- likely -- because you're mutating a mutable array from different threads. That sounds like the right answer to me - I'd delete my answer but I will leave it here as I think my comments are worth something (yet not 100% relevant to your problem).