Why isn't my use of NSLock working?

Question

I am writing code to render and rotate a picture whose details are being simultaneously calculated and updated. It works error-free on a single thread (with a display link), but looks clunky, and I don't want the calculations to be triggered by the display link. So I want to execute all the OpenGL-related code in the main thread (with the display link) and all the calculations in a second thread (executing a while (YES) loop).

I implemented this using NSThread. It works beautifully for a while and then fails with 'Thread 1: Program received signal: "EXC_BAD_ACCESS"' during glDrawArrays, and sometimes has weird flashes of graphics. This is what I expected if the main thread reads the model-level data at the same time the second thread was overwriting it.

I then defined an NSLock in the model object and locked it for all writing (in my model class) and reading (in my view class)... but it can still result in the same error, and the graphics still occasionally has weird flashes.

Have I done something wrong here, or is my problem somewhere else?

Secondly, what is the right way to stop the second thread in this case? The NSThread class reference suggests using cancel, checking isCancelled, and exiting if so, but it also says invoking exit should be avoided.

Here are the modifications to the code - in my controller class (I'm using XCode 4.2 with ARC; all my ivars are nonatomic):

@interface MyController : NSObject {
    NSThread *calcThread;
    ...
}
// (I do not give it an @property or @synthesize line)

@implementation MyController
- (void) awakeFromNib {
    calcThread = [[NSThread alloc] initWithTarget:self 
            selector:@selector(calcLoop:) object:nil];
    [calcThread start];
    ...
}
- (void) calcLoop:(id)arg { 
    @autoreleasepool {
        while (YES)
            [myModel calculate];
    }
}
...

I put the NSLock in my model class:

@interface MyModel : NSObject {
    NSLock* oLock;
    ...
}
@property (nonatomic, strong) NSLock* oLock;

@implementation myModel
-(id) init {
    oLock = [[NSLock alloc] init];
    ...
}
-(void) changeModelAppearance {
    [oLock lock];
    ...
    [oLock unlock];
}
...

and in my view class:

@implementation MyView
-(void) modelUpdated:(NSNotification *) notification {
// modelUpdated is called via the NSNotificationCenter
    MyModel* myModel = (MyModel*) [notification object];
    [myModel.oLock lock];
    ... // update OpenGL structures with data from myModel
    [myModel.oLock unlock];
}
...

Thanks!

Answer 1

I think you would have a much easier time using grand central dispatch in this case.

@interface MyController : NSObject { // Not use why you're not inheriting from NSController here.
    dispatch_queue_t calQueue;
    ...
}

- (void) awakeFromNib {
    calcQueue = dispatch_queue_create("com.yourApp.calc", DISPATCH_QUEUE_SERIAL);
    dispatch_async(calcQueue, ^{
        while(YES) // This will peg the CPU at 100%
            [myModel calculate];
    });
}

model class

@interface MyModel : NSObject {
    dispatch_queue_t modelQueue;
    ...
}
@property dispatch_queue_t modelQueue;

@implementation myModel
-(id) init {
    modelQueue = dispatch_queue_create("com.yourApp.model", DISPATCH_QUEUE_SERIAL);
}

-(void) dealloc {
    dispatch_release(modelQueue);
}

-(void) changeModelAppearance {
    dispatch_async(modelQueue, ^{
        ...
    });
}
...

View

@implementation MyView
-(void) modelUpdated:(NSNotification *) notification {
// modelUpdated is called via the NSNotificationCenter
    MyModel* myModel = (MyModel*) [notification object];
    dispatch_async(model.modelQueue, ^{
        ... // update OpenGL structures with data from myModel 
    });
}
...

To pause any of the Queues you just call dispatch_suspend and to restart any queue use dispatch_resume

If you use a timer instead of the an infinite loop you can reduce the amount of CPU you are using.

calcTimer = dispatch_source_create(DISPATCH_SOURCE_TYPE_TIMER, 0, 0, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0));
dispatch_source_set_timer(calcTimer, DISPATCH_TIME_NOW, DT, 1000);
dispatch_source_set_event_handler(calcTimer, ^{
    ...
});
dispatch_resume(calcTimer);

This would use much lower cpu overhead.