Wrapping NSArray manipulations with '@synchronized', still crash

Question

I have following class where I have several methods that manipulate with cachedRequests array

MyClass.h

@interface MyClass : NSObject {
    NSMutableArray *cachedRequests;    
}
+(MyClass*) instance; //Singleton

MyClass.m

@interface MyClass ()
- (void) sendFromCache:(CacheData*)data;
@end

@implementation MyClass

static MyClass *sharedSingleton = nil;

+(MyClass*) instance{
@synchronized(self) {
    if(sharedSingleton == nil)
        sharedSingleton = [[super allocWithZone:NULL] init];
  }
  return sharedSingleton;
}

   - (void) initialize 
   { 
        cachedRequests = nil;
    }

   - (void) processCache
  {   
       cachedRequests = [self getCachedRequests];
  }    

  - (void) sendNext
 {       
  @synchronized (self)
  {

    if (cachedRequests != nil && [cachedRequests count] == 0){
        return;
    }
    CacheData *data = [cachedRequests objectAtIndex:0]; // <-- here I get Crash 


    if (data != nil) {
        [cachedRequests removeObject:requestData];
    }
  }
}


@end

Looks like when I call: CacheData *data = [cachedRequests objectAtIndex:0]; some other thread resets cachedRequests and I get this crash.

So what I did is:

   - (void) initialize 
   { 
      @synchronized (self){
        cachedRequests = nil;
      }
    }

The question are:

is it enough? Do I need to add @synchronized (self) for cachedRequests = [self getCachedRequests];
is it good practice to use: @synchronized (cachedRequests) instead?

Crash details:

2   CoreFoundation                  0x18595af68 -[__NSArrayM objectAtIndex:] + 240 (NSArray.m:410)
3   living                          0x100c56a1c -[RequestCache sendNext] + 168

score 1 · Accepted Answer · answered May 17 '17 at 09:24

1

Your variable cachedRequests is a global variable and shared by all instances of MyClass, so different intances are synchronising on different self objects to access the same array - this could easily be the source of your crash.

You probably meant to declared an instance variable by declaring it within braces:

@implementation MyClass
{
    NSMutableArray *cachedRequests;
}

You should also, as you suspect, protect each access to the variable.

HTH

answered May 17 '17 at 09:24

CRD

52,522
5
70
86

updated, thank you, ` NSMutableArray *cachedRequests;` I declare in header – snaggs May 17 '17 at 09:32
So what question remains? Are you still getting crashes with `cachedRequests` being an instance variable and after protecting each use of it with `@synchronized`? – CRD May 17 '17 at 09:41
I protected with `@synchronized` only `sendNext` method as mentioned above. But still got crash. So now I wrapped `@synchronized` at `cachedRequests = nil;` My class is Singleton. Please see questions I asked. Thanks – snaggs May 17 '17 at 09:46
As the answer says you must "protect each access to the variable" - *all* instance methods which access the instance variable must have exclusive access to it or else there is no exclusive access! – CRD May 17 '17 at 09:52
BTW It is better to declare the instance variable in the implementation, as shown in the answer, rather than in the header, as shown in your edited question. Instance variables declared in the header can easily be accessed from outside the implementation, which in this case would bypass your threading protection. Instance variables declared in the implementation cannot be directly accessed from outside. – CRD May 17 '17 at 17:26

Wrapping NSArray manipulations with '@synchronized', still crash

1 Answers1