views:

212

answers:

1

I am trying to launch a background thread to retrieve XML data from a web service. I developed it synchronously - without threads, so I know that part works. Now I am ready to have a non-blocking service by spawning a thread to wait for the response and parse.

I created an NSAutoreleasePool inside the thread and release it at the end of the parsing. The code to spawn and the thread are as follows:

Spawn from main-loop code:

 .
 .
 [NSThread detachNewThreadSelector:@selector(spawnRequestThread:) 
                          toTarget:self withObject:url];
 .
 .

Thread (inside 'self'):

-(void) spawnRequestThread: (NSURL*) url  {

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

 parser = [[NSXMLParser alloc] initWithContentsOfURL:url];

 [self parseContentsOfResponse];

 [parser release];
 [pool release];
}

The method parseContentsOfResponse fills an NSMutableDictionary with the parsed document contents. I would like to avoid moving the data around a lot and allocate it back in the main-loop that spawned the thread rather than making a copy. First, is that possible, and if not, can I simply pass in an allocated pointer from the main thread and allocate with 'dictionaryWithDictionary' method? That just seems so inefficient.

parseContentsOfResponse

-(void)parseContentsOfResponse {

    [parser setDelegate:self];
    [parser setShouldProcessNamespaces:YES];
    [parser setShouldReportNamespacePrefixes:YES];

    [parser parse];

    NSError *parseError = [parser parserError];
    if (parseError) {
        NSLog(@"%@", parseError);
        NSLog(@"publicID: %@", [parser publicID]);
        NSLog(@"systemID: %@", [parser systemID]);
        NSLog(@"line:%d column:%d", [parser lineNumber], [parser columnNumber]);
    }

    ready = YES;
}

First parse section

Each section creates element strings when its elementStart is signaled. The elementEnd will add the object to the dictionary and release the element. The remaining details are redundant and I think the point to note is that the allocations are not directed at an NSZone, therefore they should be residing in the thread's memory pool.

- (void)parserDidStartDocument:(NSXMLParser *)parser {
    NSLog(@"%s", __FUNCTION__);
    currentChars    = [NSMutableString stringWithString:@""];
    elementQuestion = [NSMutableString stringWithString:@""];
    elementAnswer   = [NSMutableString stringWithString:@""];
    elementKeyword  = [NSMutableString stringWithString:@""];
}
+1  A: 

The simplest thing to do would be to create the dictionary in the separate thread, then set it as a property on the main thread, like so:

- (void)spawnRequestThread: (NSURL*) url  {
    NSMutableDictionary *dict = [[NSMutableDictionary alloc] init];
    //do stuff with dict
    [self performSelectorOnMainThread:@selector(doneWithThread:) withObject:dict waitUntilDone:NO];
}

- (void)doneWithThread:(NSDictionary *)theDict {
    self.dict = theDict; //retaining property, can be an NSDictionary
}

Do you need to change the contents of the dictionary over time? If so, allocating on the main thread and changing the contents in the other thread is possible, but you have to worry about thread-safety issues--NSMutableDictionary isn't thread-safe, so you'd have to use an atomic property and locks:

//.h
@property (retain) NSMutableDictionary *dict; //don't use "nonatomic" keyword
@property (retain) NSLock *dictLock;

//.m
- (id) init {
    //blah blah
    dict = [[NSMutableDictionary alloc] init];
    dictLock = [[NSLock alloc] init];
    return self;
}

- (void)spawnRequestThread: (NSURL*) url  {
    //whenever you have to update the dictionary
    [self.dictLock lock];
    [self.dict setValue:foo forKey:bar];
    [self.dictLock unlock];
}

Locking is quite expensive and inefficient in any case, so I'd tend to prefer the first method (I'm not sure which is more expensive, exactly, but the first is simpler and avoids thread-safety issues).

Also, looking at your code, it looks like your NSXMLParser is an ivar which you directly access. This is a bad idea, since NSXMLParser isn't thread-safe--I would recommend implementing it as a local variable instead. If you do need it as an ivar, use an atomic property and locks and only access it through accessors.

eman
Great --- so performSelectorOnMainThread will get me back from the background thread to the main thread so I can deposit my dictionary updates into its memory space -- is that correct understanding of your recommendation?
mobibob
Yes, I will eventually have 'updates' to the dictionary. For now, it is a blob at the beginning of the application. The next step is to save to the user's space, reload and check for updates. So, I will have to manage the locking.
mobibob
Good advice on the NSXMLParser as an ivar. I think I can easily put it in the local scope with my refactoring to the threading model. Meanwhile, I am only using the accessors -- but I can't be trusted to remember that in the future as I modify the app :)
mobibob
Yes, `performSelectorOnMainThread` puts you back on the main thread (it's the same memory space for all threads, but some operations are unsafe from threads other than the main thread, such as accessing non-threadsafe objects or updating the UI). Come to think of it, the second method in the answer is still not safe--you'd still have to implement locks whenever you change the dictionary's contents (I'll update the example).
eman
Darn it -- now I have to think :( I really wanted to avoid locking and rely on modal views and an atomic 'ready' flag. This is very helpful and educational. I hope your answer gets a lot of up-votes.
mobibob