1

Objective-C weak properties should point to nil if the object gets deallocated, but in this case weak properties seem to retain the object. Consider the case:

@interface SillyObject : NSObject

@property (nonatomic, assign) NSInteger val;

-(void)printVal;

@end

@implementation SillyObject

-(void)printVal
{
  NSLog(@"%d", self.val);
}

@end

-(void)saveReference
{
  SillyObject* s = [SillyObject new];
  s.val = 100;

  [[ObjectCache sharedInstance] addWeakRef:s callback:^(NSString* junk) {
    [s printVal];
  }];
}

callSillyObjectBlocks loops over all the objects added to the cache and calls the correspondending blocks (see below)

-(void)callDeadObject
{
  [self saveReference];
  [[ObjectCache sharedInstance] callSillyObjectBlocks];
}

Now saveReference exits and the SillyObject should be deallocated, but it doesn't and the weak reference is not nil.

The relevant implementation details of the cache:

typedef void (^Callback)(NSString* junk);

@interface CacheSlot : NSObject

@property (nonatomic, copy) Callback callback;
@property (nonatomic, weak) id source;
// More irrelevant properties.

-(instancetype)initWithRef:(__weak id)obj callback:(Callback)cb;

@end

@implementation CacheSlot

@synthesize callback, source;

-(instancetype)initWithRef:(__weak id)obj callback:(Callback)cb
{
  self = [super init];
  if(self)
  {
    self.callback = cb;
    self.source = obj;
  }
  return self;
}

@end

@interface ObjectCache()

// This array contains CacheSlot objects
@property (nonatomic, strong) NSMutableArray* savedObjects;
@end

// Implementation.
-(void)addWeakRef:(id)obj callback:(Callback)block
{
  __weak id src = obj;
  [self.savedObjects addObject:[[CacheSlot alloc] initWithRef:src callback:block]];
}

-(void)callSillyObjectBlocks
{
  for(CacheSlot* slot in self.savedObjects)
  {
    if(slot.source)
    {
      slot.callback(@"Whatever!");
    }
    else
    {
      // Remove the slot from cache
    }
  }
}

Calling saveReference initially should create a temporary object which gets deallocated as soon as the function exits (which it does if I call addWeakRef:nil instead).

After calling saveReference I run callSillyObjectBlocks and the added object's corresponding block should not be called, but it gets called with the object's value. Output:

100
Gabriele Petronella
  • 106,943
  • 21
  • 217
  • 235
SiimKallas
  • 934
  • 11
  • 23
  • Note that reading a weak variable may retain and autorelease the object. You may need to add an autorelease pool to clean up those references and allow the object to die. – Greg Parker Oct 04 '13 at 20:02

1 Answers1

3

Whenever you have a block that references a variable from outside the block, you need to declare it as weak as well. Otherwise you get a strong reference. So it should be:

-(void)saveReference
{
  SillyObject* s = [SillyObject new];
  s.val = 100;
  SillyObject * __weak weakSilly = s;

  [[ObjectCache sharedInstance] addWeakRef:s callback:^(NSString* junk) {
    [weakSilly printVal];
  }];
}

At the same time you could simplify your code by removing __weak from a few other locations as they affect method arguments only that will no retain an instance for a relevant time:

-(instancetype)initWithRef:(id)obj callback:(Callback)cb {
    ...
}

-(void)addWeakRef:(id)obj callback:(Callback)block
{
  [self.savedObjects addObject:[[CacheSlot alloc] initWithRef:obj callback:block]];
}
Codo
  • 75,595
  • 17
  • 168
  • 206
  • Thanks, although in an usability way this is terrible :( If I'm using the cache and want to add self to the object cache with a corresponding block, I wouldn't want to exchange self with weakSelf in every line of the block. But at the moment I can't think of a workaround for this. – SiimKallas Oct 04 '13 at 13:15
  • You might want to think about the design of your cache. As it currently stands, you add two references for each cached instance: one direct reference (instance variable *source*) and one indirect reference (via the *callback* instance variable and the associated block). One could also image that all cached instance implement a common *CacheCallback* protocol. Then you could get rid of the *callback* instance variable and all associated problems. – Codo Oct 04 '13 at 13:22
  • You are right, however using blocks would have been more convenient than than making the class conform to the protocol every time. – SiimKallas Oct 04 '13 at 13:25