Tuesday, May 31, 2016

Removing Weak Self Pointers

Blocks brought a variety of sharp edges to Objective-C. The most acute has been the threat of accidental retain cycles due to self references. Explicit calls to self, implicit references such as ivars, and accidental references such as in NSAssert can all easily cause an object to be held indefinitely.

The Dangerous Solution to Cycles

An unfortunately common way to avoid retain cycles in blocks has been reliance on weak self references:
 __typeof(self) __weak weakSelf = self;  
 [object prepareVacationWithBlock:^{  
  __typeof(weakSelf) strongSelf = weakSelf;  
  [strongSelf vaccinateDog];  
  [kennel reserveDogSpace];  
  [strongSelf hireCatSitter];  
 }];  
A fundamental problem with this approach is that weak self references create code that in the user’s hands may not execute at all like expected or tested.
For example, during development, it’s typical for the weakSelf in the block above to never be nil. But in a release build, objects may be more aggressively released, so the weakly-referenced object may be released before the block executes. When the example block then runs on the user’s device, strongSelf is occasionally nil, and kennel is getting reserved even though the dog didn’t get vaccinated.
Blocks retain references to ensure that each block’s dependencies are still around when the block code executes. Reliance on weak self references subverts these semantics, making the code that actually runs in the block unpredictable. Use of weak selfs turns a typically minor problem, a small retain cycle that ought to be caught in Instruments during app development, into a much more serious problem of untested code paths that crop up just on end-user machines. A small, predictable leak is replaced by an elusive crash or hang.
Use of weak self references has been dressed up with “weakify” macros. Those formalize the approach of ignoring the problem rather than fixing it.

Safety by Design

So what’s a better approach? Design APIs that cannot cause retain cycles. Class properties and method parameters should never take a block pointer unless the API provider can guarantee there won’t be a retain cycle created. Safe use patterns should be inherent in the interface; they shouldn’t be left up to the understanding or paranoia of every client trying to use the API.
When blocks passed to a class can’t be guaranteed to not cause retain cycles, then the class should have zeroing weak object pointers for its callbacks, not block pointers. It is the class’s responsibility to vend a safe API, rather than a challenge for every caller to try to get right. Blocks are often chosen for callbacks for the short-term design advantage, ignoring the long-term cost they impose on client code.
Still, some designs unfortunately do require blocks. The simplest way a method can ensure that blocks passed as parameters won’t lead to retain cycles in the client code is to only hold blocks in local variables. Since blocks pointed to from the stack are certain to be released as the stack unwinds, they’re guaranteed to release their references as well.
Often, a class will have to hold on to a block for use later, either as a completion handler or for repeated use as a monitoring callback. In either case, the class should be written to ensure that all blocks are released at a specific time in the near future. The example class below does that.
Waiting for the class to deallocate to release its blocks just leaves class’s users in a battle against retain cycles.


 @interface CatVideoDownloader : NSObject <CatVideoFetcherDelegate>  
   
 - (void)fetchCatVideoWithCompletionHandler:  
   (void (^)(NSData *kittenData, NSError *error))handler;  
   
 // The optional progress block may be called repeatedly.  
 @property(nonatomic, copy)   
   void (^progress)(uint64_t numberOfBytesReceived);  
 @end  
   
 @implementation CatVideoDownloader {  
  // We'll hold this completion handler only until the fetcher  
  // finishes or fails. When either happens, we call back to  
  // the client appropriately, and release all block ivars.  
  void (^_completionHandler)(NSData *kittenData, NSError *error);  
 }  
   
 - (void)fetchCatVideoWithCompletionHandler:  
   (void (^)(NSData *kittenData, NSError *error))handler {  
  // Start downloading with this object as the fetcher's delegate.  
  _completionHandler = [handler copy];  
  [CatVideoFetcher startFetchingCatVideoWithDelegate:self];  
 }  
   
 - (void)fetcher:(CatVideoFetcher *)fetcher  
   didReceiveBytes:(uint64_t)numberOfBytesReceived {  
  // Update the client on progress.  
  if (_progress) {  
   _progress(numberOfBytesReceived);  
  }  
 }  
   
 - (void)fetcher:(CatVideoFetcher *)fetcher  
   finishedWithData:(NSData *)kittenData  
         error:(NSError *)error {  
  // Downloading finished or failed; call back and release blocks.  
  [self invokeCallbackAndCleanUpWithData:kittenData  
                   error:error];  
 }  
   
 - (IBAction)userPressedCancel:(id)sender {  
  // User cancelled; call back and release blocks.  
  NSError *error =   
   [NSError errorWithDomain:@"com.example.CatVideoDownloader"  
             code:NSURLErrorCancelled  
           userInfo:nil];  
  [self invokeCallbackAndCleanUpWithData:nil error:error];  
 }  
   
 - (void)invokeCallbackAndCleanUpWithData:(NSData *)kittenData  
                   error:(NSError *)error {  
  // Ensure the handler ivar is nil before calling the handler  
  // so reentrancy won't lead to the callback being called twice.  
  void (^callback)(NSData *, NSError *) = _completionHandler;  
  _completionHandler = nil;  
  if (callback) {  
   callback(kittenData, error);  
  }  
   
  // We’re done, so release all other blocks too.  
  _progress = nil;  
 }  
   
 @end  

3 comments:

  1. Hi Greg! Nice article. I'd like to see your equivalent CatVideoFetcher done with "zeroing weak object pointers for its callbacks, not block pointers." for comparison.

    ReplyDelete
  2. What in particular would be interesting to see? I think the code would look similar, aside from the class taking a pointer (held weakly) and selector for the callback. It's just not as critical that a weak pointer be explicitly zeroed as early as possible.

    ReplyDelete
  3. Sorry, I just don't understand the pattern - it's probably obvious, and probably it's just me.

    ReplyDelete