Improving on the common AnyCancellable .store(…) pattern.

If you’ve been reading any Combine documentation or code, you’ve probably seen code like this before:

let newPhotos = photos.selectedPhotos
newPhotos
  .map { [unowned self] newImage in
    return self.images.value + [newImage]
  }
  .assign(to: \.value, on: images)
  .store(in: &subscriptions)

Pretty standard: call something that returns a publisher, map the results to whatever type you need, assign that value to an instance variable, and finally, store the resulting AnyCancellable into a Set named subscriptions which is an instance variable or property in the enclosing scope. In this case we’re in a UIViewController subclass where it’s defined as:

private var subscriptions = Set<AnyCancellable>()

This code is from the popular book on Combine.

The problem with this pattern in this particular case, and other similar situations, is that the AnyCancellable is never removed from the subscriptions Set. Sometimes this doesn’t matter – when the enclosing scope is going to go away and the code is only called once relatively soon before exiting scope cleans up memory. But sometimes, as in the sample project this code is from, the user can do the action invoking this code (adding photos in this case) repeatedly. Each time this code is invoked the .store(...) call adds another AnyCancellable to the Set. Since this view controller is the root view controller of the app it is never closed and this memory is never cleaned up during the lifetime of the app.


An easy solution is to add an instance variable to your class (a UIViewController in this case):

private var newPhotosSubscription: AnyCancellable?

and then change the code above to something like this so the AnyCancellable is assigned to the instance variable and the .store(..) part is removed:

self.newPhotosSubscription = newPhotos
  .map { [unowned self] newImage in
    self.images.value + [newImage]
  }
  .assign(to: \.value, on: images)

When this assignment happens it frees the previous value, if any, and thus there will be at most a single AnyCancellable kept around.

Ok, that works. But I’m a perfectionist and that single AnyCancellable hanging around bugged me. That’s fixable, but as I went and fixed this issue in more places in the project there was a proliferation of instance variables as I had to add one for each place this came up.

So here’s a solution I came up with to avoid that.

First, add an extension on AnyCancellable:

extension AnyCancellable {
  func store(in dictionary: inout [UInt64: AnyCancellable], 
             for key: UInt64) {
    dictionary[key] = self
  }
}

Then change the subscriptions instance variable to be a matching Dictionary (and remove the individual instance variables you added for each case previously, if you did that already):

private var subscriptions = Dictionary<UInt64, AnyCancellable>()

In our original function which created the newPhotosSubscription and let the user choose photos, change the code to:

let key = DispatchTime.now().uptimeNanoseconds
 
newPhotos
  .handleEvents(receiveCompletion: { [unowned self] _ in
    self.subscriptions.removeValue(forKey: key)
  })
  .map { [unowned self] newImage in
    self.images.value + [newImage]
  }
  .assign(to: \.value, on: images)
  .store(in: &subscriptions, for: key)

So above we created a Dictionary instead of a Set for our subscriptions instance variable that holds on to the AnyCancellable so it remains allocated while needed. The dictionary allows us to store the AnyCancellable under a unique key, which in this case is a UInt64. On the first line above we create that key and assign it the number of nanoseconds since the device rebooted.

Then we add a .handleEvents operator to the subscription pipeline. Once the publisher has sent the .finished or .failure completion event we no longer need to keep the AnyCancellable around. Our receiveCompletion closure code removes the AnyCancellable stored under the captured key from our subscriptions instance variable.

(Note: if you preferred, you could replace UInt64 with UUID in the dictionary declaration and the AnyCancellable extension. Then instead of setting the key value to DispatchTime.now().uptimeNanoseconds you could generate a new UUID with UUID(). All that matters is that the is a unique value conforming to Hashable and doesn’t cost too much to generate).


I’m relatively new to Combine, so if you know a better way to do this, I’d love to hear about it!


Addendum

1) Be aware that Swift closures capture variables by reference by default (unlike for Objective C). So don’t re-use the key variable like I did in my project 

2) I also ran into a case where the pattern above did odd things for a Combine pipeline subscribed to a Future publisher – the store seems to happen after the pipeline has finished executing (huh?!) and thus the clean up in the completion handler is called before the store. I haven’t dug into that to understand why, but thought I’d mention it in case you are seeing things not get cleaned up when you expect like I was.

-> I later learned that this is because in Combine Future is “eager” and will complete immediately. One option is to wrap it in a Deferred publisher and another is to capture the AnyCancellable in a local variable and only save it to the dictionary if the completion block hasn’t been called yet.

Advertisement

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

%d bloggers like this: