Search code examples
iosobjective-creactive-cocoa

RAC and cell reuse: putting deliverOn: in the right place?


I was playing with RAC and, in particular, Colin Eberhardt's Twitter search example, and came across a crash that I could not explain to myself.

Here is a sample project I have created to illustrate the issue and base the question on.

The app uses a UITableView with reusable cells; each cell has a UIImageView on it whose image is downloaded by some URL.
There is also defined a signal for downloading an image on a background queue:

- (RACSignal *)signalForLoadingImage:(NSString *)imageURLString
{
    RACScheduler *scheduler = [RACScheduler
                               schedulerWithPriority:RACSchedulerPriorityBackground];

    return [[RACSignal createSignal:^RACDisposable *(id<RACSubscriber> subscriber) {
        NSData *data = [NSData dataWithContentsOfURL:[NSURL URLWithString:imageURLString]];
        UIImage *image = [UIImage imageWithData:data];
        [subscriber sendNext:image];
        [subscriber sendCompleted];
        return nil;
    }] subscribeOn:scheduler];
}

In cellForRowAtIndexPath:, I bind the loading signal to the image view's image property with RAC macro:

RAC(cell.kittenImageView, image) =
[[[self signalForLoadingImage:self.imageURLs[indexPath.row]]
  takeUntil:cell.rac_prepareForReuseSignal]      // Crashes on multiple binding assertion!
 deliverOn:[RACScheduler mainThreadScheduler]];  // Swap these two lines to 'fix'

Now, when I run the app and start scrolling the table view up and down, the app crashes with the assertion message:

Signal <RACDynamicSignal: 0x7f9110485470> name:  is already bound to key path "image" on object <UIImageView: <...>>, adding signal <RACDynamicSignal: 0x7f9110454510> name:  is undefined behavior

However, if I wrap the image loading signal into deliverOn: first, and then into takeUntil:, the cell reuse will work just fine:

RAC(cell.kittenImageView, image) =
[[[self signalForLoadingImage:self.imageURLs[indexPath.row]]
  deliverOn:[RACScheduler mainThreadScheduler]]
 takeUntil:cell.rac_prepareForReuseSignal];  // No issue

So my questions are:

  1. How to explain why the latter works and the former doesn't? There's obviously some race condition causing a new signal bind to the image property before the existing one completes, but I'm totally not sure how exactly it occurs.
  2. What should I remember about to avoid such kind of subtleties in my RAC-powered code? Am I missing some basic principle in the code above, or is there any rule of thumb to apply (assuming there's no bug in RAC itself, of course)?

Thanks for reading up to here :-)


Solution

  • I haven't confirmed this, but here's a possible explanation:

    1. Cell X gets called into use, starts downloading an image.
    2. Cell X scrolls offscreen before the image download has completed.
    3. Cell X gets reused, prepareForReuse is called.
    4. Cell X's rac_prepareForReuseSignal sends a value.
    5. Because of deliverTo:, the value is dispatched to the main queue, introducing a runloop delay. Of note, this prevents synchronous/immediate unbinding of the image property.
    6. Cell X is back being used cellForRowAtIndexPath:
    7. New image binding is invoked and causes warning
    8. … next runloop …
    9. Original binding is finally now destructed, but it's a bit too late.

    So basically the signal should be unbound between 4 and 6, but the -deliverTo: reorders the unbinding to come later.