I have an Objective-C class with an NSMutableDictionary as an instance variable. The dictionary is occasionally read from one thread as it’s being mutated on another thread, causing a crash. My class looks like this:

@interface MyClass: NSObject {
    NSMutableDictionary sharedState;
}
@end

@implementation MyClass

- (instancetype) init {
    self = [super init];
    if (self != nil) {
        [self reinitialize];
    }
    return self;
}

- (void) reinitialize {
    sharedState = [[NSMutableDictionary alloc] init];
}

@end

There are other methods that read from or mutate sharedState. The reinitialize method is the only place where the pointer is actually assigned to, but there are methods other than the initializer that call reinitialize.

In order to solve my threading problems, I was thinking of wrapping all uses of sharedState, including the one in reinitialize, in

@synchronized (sharedState) {
    ...
}

since it’s straightforward and the performance hit shouldn’t be a problem in practice. However, in reinitialize in particular, I would be synchronizing on an instance variable that has not even been alloced yet, and on subsequent calls I would be reassigning the pointer on which I’m synchronizing. Is this valid and safe? Or should I be synchronizing on self instead?

CodePudding user response：

Some thoughts here:

1. iVars are nil by default when an object is allocated, so you'll gonna be doing @synchronized(nil), which might interfere with other parts of your code which do similar things, which might create race conditions in another parts of your code
2. initializers don't need synchronization, as you can't have race conditions on a newly created object.
3. You cannot, actually you should not, @synchronize on an object that you'll gonna be replacing in the @synchronized block, this will nullify your attempts to avoid data races

If you want to protect members of your class, then yes, indeed, it's better and safer to @synchronizd(self), the rule here is to synchronize one layer above the one of the value you want to protect, unless you are 101% sure that the pointer will never ever change, in that case you can synchronize over the member, however this doesn't seem to be the case in your code.

Just make sure you wrap all accesses to sharedState, this is actually the main challenge, to make sure that new methods you add don't create race conditions on their own.

CodePudding user response：

@synchronized (nil) should block nothing but is still a possible semaphore because when the semaphore is nil it has still an address that is unique. In case of nil just not unique enough. For which i would assume it can lead to undefined behaviour.

NSObject *object = nil;
@synchronized (object) {
   NSLog(@"address? %p", object);
}

The idea of the semaphore is to provide some unique identifier.

The docs just say.

It prevents different threads from acquiring the same lock at the same time

and

To protect sections of code from being executed by more than one thread at a time, Objective-C provides the @synchronized() directive.

The @synchronized() directive locks a section of code for use by a single thread. Other threads are blocked until the thread exits the protected code—that is, when execution continues past the last statement in the @synchronized() block.

you can also create your own semaphore and it still works.

static void *somevalue = &somevalue;
@synchronized (somevalue) {
   // one thread works on this block of code at one time.
}

It does not lock the value, it locks the enclosed block of code from being accessed from other threads until the block is left/unlocked.

But when you use self as mutex/mutual exclusion semaphore you actually lock the class because only one class object exists that is shared from all objects.

So in case you want to be super safe that no other method can work on the same address space you could indeed use @synchronized (self) that lets only one method of the class work at the process time of the synchronized block.

But as your object is an internal member of your class you can also provide a proper getter method to make outside access safe, in example by copying. Depends on the use case if that fits your needs.

-(NSMutableDictionary*)sharedStateCopy {
    return [sharedState copy]; //actually [.. mutableCopy]
}