CODEWITHSUNDEEP
objective-cgrand-central-dispatch
p0lAris
p0lAris

Reputation: 4820

Objective c Thread/Runloop/Concurrency consideration

Consider the following Class:

@interface TaskScheduler ()

@property (strong) NSMutableDictionary *tasks;

@end

@implementation TaskScheduler

- (void)addTask:(Task *)task
{
    [_tasks setObject:task forKey:task.id];
}

- (void)cancelTask:(NSString *)id
{
    [_tasks removeObjectForKey:id];
}

- (void)runTask:(Task *)task
{
    // run task in a background concurrent global dispatch queue
    dispatch_queue_t backgroundConcurrentQueue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_BACKGROUND, 0);

    void (^dispatchBlock)() = ^void(){
        BOOL success = task.taskBlock(); // typedef BOOL (^TaskBlock)();
        if (success)
        {
            [self cancelTask:task.id];
        }
    };
    dispatch_async(backgroundConcurrentQueue, dispatchBlock);
}

- (void)didUpdateSystemUpdateValue
{
    // some other class has a `dispatch_source_t` timer that fires every second and calls this delegate API

    if (shouldRunTask)
    {
        for (Task *task in _tasks.allValues)
        {
            [self runTask:task];
        }
    }
}

@end

Now please notice how I am cancelling a task inside the dispatch queue block call itself.

I am a bit confused here — is there any issue in the runTask: call? I am cancelling the task if it succeeds inside the dispatchBlock that runs using the global dispatch queue. Only tasks inside tasks can run.

The only issue I can see that the same task can be run multiple times if some condition holds true unless the task succeeds in one of the dispatch calls after which it will won't exist in the queue (or a dictionary of tasks).

EDIT: I have made changes to the original question. I never intended that the rest of the design would need to be part of the question. The question can be answered without the newest changes but just in case.

Upvotes: 0

Views: 85

Answers (1)

ipmcc
ipmcc

Reputation: 29946

You appear to be assuming that it's safe for -setObject:forKey: and -removeObjectForKey: to be called concurrently from two different threads, and it is categorically not safe. You need additional synchronization. The fact that you wrap the -removeObjectForKey: call inside your -cancelTask: method, and call it from your -runTask: method is irrelevant. NSMutableDictionary is not safe for concurrent operations from multiple threads no matter how many other methods you wrap it in, if none of those wrapping methods provide any synchronization which none of those copied here do.

Upvotes: 3

Related Questions