Magic Bullet Dave
Magic Bullet Dave

Reputation: 9076

Atomic property with custom getter

I have a class with a property declared as:

@property (nonatomic, strong) NSMutableArray *links;

I want to lazily instantiate it so have the following custom getter:

- (NSMutableArray *)links {

    if (!_links) {
        _links = [NSMutableArray array];
    }

    return _links;
}

My app has progressed somewhat and the object can now be accessed from different threads. I change the declaration to:

@property (atomic, strong) NSMutableArray *links;

This generates a compiler warning: Writeable atomic property cannot combine a synthesised setter with a user defined getter.

Which I understand - I think. My question is, it is correct to do the following, in order to create an atomic property with a custom getter?:

EDIT: this is the code for my new custom setter and getter:

- (NSMutableArray *)links {

   if (!_links) {
       @synchronized(self) {
           if (!_links) {
               _links = [NSMutableArray array];
           }
       }
   }

   return _links;    
}

- (void)links:(NSMutableArray *)links {
       @synchronized(self) {
         _links = links;
    }
 }

Upvotes: 4

Views: 2075

Answers (2)

gnasher729
gnasher729

Reputation: 52538

I wonder why you want a setter. And I wonder why you want the property to be atomic.

The easiest and correct way, if you want a getter and a setter, is to write

- (NSMutableArray *)links{
    @synchronized (self)
    {
        if(!_links){
            _links = [NSMutableArray.alloc init];
        }
    }
    return _links;
}

- (void)setLinks:(NSMutableArray *)links{
    @synchronized(self)
    {
        _links = links;
    }
}

Upvotes: 0

Ken Thomases
Ken Thomases

Reputation: 90571

First, just because the property may be accessed from multiple threads doesn't mean that a) it needs to be atomic, nor that b) making it atomic is sufficient to make it thread-safe.

In particular, the property type is NSMutableArray*, which means that any caller gets the mutable array and can mutate it without restriction, which is inherently thread-unsafe. You should basically never make a property with mutable type. All changes to the property should have to go through accessor methods so you can control or, at a minimum, react to changes.

Second, your getter is using the double-checked lock anti-pattern. It is not safe in C-based languages like Objective-C. Don't do that.

That said, you're correct that if you make a property atomic and you implement either the getter or the setter, then you must implement both and you are also responsible for implementing the synchronization to enforce the atomicity. Using @synchronized() is suitable, so long as you don't use the double-checked locking approach.

But you really need to make your class thread-safe with respect to that property (and any others that can be accessed from multiple threads), which requires more than making the property atomic.

Upvotes: 5

Related Questions