swift combine sink receiveValue memory leak

Question

I'm having a hard time with dealing with Combine. After the publisher is complete I want to update a value but whenever I update that value the memory is allocated and never goes away.

Whenever I try to assign image there is a leak. If I don't assign no leak.

EDIT: Reproducible example here: https://github.com/peterwarbo/MemoryAllocation

This is what my code looks like:

final class CameraController: ObservableObject {

    private var storage = Set<AnyCancellable>()    
    var image: UIImage?

    func capture(_ image: UIImage) {

        PhotoLibrary.saveImageToTemporaryDirectory(image) // AnyPublisher<URL, Error>
            .zip(PhotoLibrary.saveImage(image, location: self.locationObserver.location) // AnyPublisher<UIImage, Error>)
            .sink(receiveCompletion: { [weak self] (completion) in
                switch completion {
                case let .failure(error):
                    Log.error(error)
                    self?.handleCaptureError(error)
                case .finished: break
                }
            }) { [weak self] (value) in
                print(value.1) // no leak
                self.image = value.1 // leak

            }
            .store(in: &self.storage)
     }
}

I've also tried instead of using sink:

.receive(
    subscriber: Subscribers.Sink(
        receiveCompletion: { [weak self] completion in
            switch completion {
            case let .failure(error):
                Log.error(error)
                self?.handleCaptureError(error)
            case .finished: break
            }
        },
        receiveValue: { value in
            print(value.1) // no leak
            self.image = value.1 // leak            
        }
    )
)

Answer 1

An obvious problem with your code is that you create and store a new pipeline every time capture is called. That is the opposite of how to use Combine; you might as well not be using Combine at all. The way to use Combine is to create a pipeline once and let information come down the pipeline asynchronously from then on.

You posted an example project in which you use a Future to introduce a delay in the passing of an image down a pipeline. In your project, the user chooses an image from the photo library, repeatedly. Once again, in your project you create and store a new pipeline every time an image is chosen. I rewrote the example as follows:

import UIKit
import Combine

class ViewController: UIViewController, UINavigationControllerDelegate {
    let queue = DispatchQueue(label: "Queue", qos: .userInitiated, attributes: [], autoreleaseFrequency: .workItem)
    var image: UIImage?
    var storage: Set<AnyCancellable> = []
    let publisher = PassthroughSubject<UIImage, Never>()
    override func viewDidLoad() {
        super.viewDidLoad()
        self.publisher
            .flatMap {image in
                self.futureMaker(image: image)
            }
            .receive(on: DispatchQueue.main)
            .sink(receiveCompletion: { (completion) in
            }) { (value) in
                print("finished processing image")
                self.image = value
            }
            .store(in: &self.storage)
    }
    @IBAction func didTapPickImage(_ sender: UIButton) {
        let picker = UIImagePickerController()
        picker.delegate = self
        present(picker, animated: true)
    }
    func futureMaker(image: UIImage) -> AnyPublisher<UIImage, Never> {
        Future<UIImage, Never> { promise in
            self.queue.asyncAfter(deadline: .now() + 0.5) {
                promise(.success(image))
            }
        }.eraseToAnyPublisher()
    }
}
extension ViewController: UIImagePickerControllerDelegate {
    func imagePickerController(_ picker: UIImagePickerController, didFinishPickingMediaWithInfo info: [UIImagePickerController.InfoKey : Any]) {
        dismiss(animated: true)
        guard let image = info[UIImagePickerController.InfoKey.originalImage] as? UIImage else { return }
        print("got image")
        self.publisher.send(image)
    }
}

Note the architecture: I create the pipeline once, in viewDidLoad, and whenever an image arrives I pass it down the same pipeline. Some memory is used, to be sure, because we are storing a UIImage; but it does not grow in any uncontrolled way, but levels off in an optimal manner.

We're using 8.4 MB after picking all the images in the library repeatedly. No problem there!

Also, no surplus large images are persisting. Looking at memory that comes from choosing in the image picker, one image persists; that is 2.7 MB of our 8.4 MB:

That is exactly what we expect.

Answer 2

Just by code reading... use weak self, not direct self,

}) { [weak self] (value) in
    print(value.1) // no leak
    self?.image = value.1     // << here !!
    self?.storage.removeAll() // just in case
}

also I would add delivery on main queue, as

PhotoLibrary.saveImageToTemporaryDirectory(image)
    .zip(PhotoLibrary.saveImage(image, location: self.locationObserver.location)
    .receive(on: DispatchQueue.main)          // << here !!
    .sink(receiveCompletion: { [weak self] (completion) in
    // ... other code here

Answer 3

You are using .store(in: &self.storage)

need to cancel this private var storage = Set()

storage.cancel() storage.removeAll()

and also self need to be weak