0

I'm stuck on a Combine problem, and I can't find a proper solution for this.

My goal is to monitor a queue, and process items in the queue until it's empty. If then someone adds more items in the queue, I resume processing. Items needs to be processed one by one, and I don't want to lose any item.

I wrote a very simplified queue below to reproduce the problem. My items are modeled as just strings for the sake of simplicity again.

Given the contraints above:

  • I use a changePublisher on the queue to monitor for changes.
  • A button lets me add a new item to the queue
  • The flatMap operator relies on the maxPublishers parameter to only allow one in-flight processing.
  • The buffer operator prevents items from being lost if the flatMap is busy.

Additionally, I'm using a combineLatest operator to only trigger the pipeline under some conditions. For simplicity, I'm using a Just(true) publisher here.

The problem

If I tap the button, a first item goes in the pipeline and is processed. The changePublisher triggers because the queue is modified (item is removed), and the pipeline stops at the compactMap because the peek() returns nil. So far, so good. Afterwards, though, if I tap on the button again, a value is sent in the pipeline but never makes it through the buffer.

Solution?

I noticed that removing the combineLatest prevents the problem from happening, but I don't understand why.

Code

import Combine
import UIKit

class PersistentQueue {
    let changePublisher = PassthroughSubject<Void, Never>()

    var strings = [String]()

    func add(_ s: String) {
        strings.append(s)
        changePublisher.send()
    }

    func peek() -> String? {
        strings.first
    }

    func removeFirst() {
        strings.removeFirst()
        changePublisher.send()
    }
}

class ViewController: UIViewController {

    private let queue = PersistentQueue()
    private var cancellables: Set<AnyCancellable> = []

    override func viewDidLoad() {
        super.viewDidLoad()
        start()
    }

    @IBAction func tap(_ sender: Any) {
        queue.add(UUID().uuidString)
    }

    /*
     Listen to changes in the queue, and process them one at a time. Once processed, remove the item from the queue.
     Keep doing this until there are no more items in the queue. The pipeline should also be triggered if new items are
     added to the queue (see `tap` above)
     */
    func start() {
        queue.changePublisher
            .print("Change")
            .buffer(size: Int.max, prefetch: .keepFull, whenFull: .dropNewest)
            .print("Buffer")
            // NOTE: If I remove this combineLatest (and the filter below, to make it compile), I don't have the issue anymore.
            .combineLatest(
                Just(true)
            )
            .print("Combine")
            .filter { _, enabled in return enabled }
            .print("Filter")
            .compactMap { _ in
                self.queue.peek()
            }
            .print("Compact")
            // maxPublishers lets us process one page at a time
            .flatMap(maxPublishers: .max(1)) { reference in
                return self.process(reference)
            }
            .sink { reference in
                print("Sink for \(reference)")

                // Remove the processed item from the queue. This will also trigger the queue's changePublisher,
                // which re-run this pipeline in case
                self.queue.removeFirst()
            }
            .store(in: &cancellables)
    }

    func process(_ value: String) -> AnyPublisher<String, Never> {
        return Future<String, Never> { promise in
            print("Starting processing of \(value)")
            DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 2) {
                promise(.success(value))
            }
        }.eraseToAnyPublisher()
    }

}

Output

Here is a sample run of the pipeline if you tap on the button twice:

Change: receive subscription: (PassthroughSubject)
Change: request max: (9223372036854775807)
Buffer: receive subscription: (Buffer)
Combine: receive subscription: (CombineLatest)
Filter: receive subscription: (Print)
Compact: receive subscription: (Print)
Compact: request max: (1)
Filter: request max: (1)
Combine: request max: (1)
Buffer: request max: (1)
Change: receive value: (())
Buffer: receive value: (())
Combine: receive value: (((), true))
Filter: receive value: (((), true))
Compact: receive value: (3999C98D-4A86-42FD-A10C-7724541E774D)
Starting processing of 3999C98D-4A86-42FD-A10C-7724541E774D
Change: request max: (1) (synchronous)
Sink for 3999C98D-4A86-42FD-A10C-7724541E774D // First item went through pipeline
Change: receive value: (())
Compact: request max: (1)
Filter: request max: (1)
Combine: request max: (1)
Buffer: request max: (1)
Buffer: receive value: (())
Combine: receive value: (((), true))
Filter: receive value: (((), true))

// Second time compactMap is hit, value is nil -> doesn't forward any value downstream.

Filter: request max: (1) (synchronous)
Combine: request max: (1) (synchronous)
Change: request max: (1)

// Tap on button

Change: receive value: (())

// ... Nothing happens

[EDIT] Here is a much more constrained example, which can run in Playgrounds and which also demonstrates the problem:

import Combine
import Foundation

import PlaygroundSupport

PlaygroundPage.current.needsIndefiniteExecution = true

func process(_ value: String) -> AnyPublisher<String, Never> {
    return Future<String, Never> { promise in
        print("Starting processing of \(value)")
        DispatchQueue.global(qos: .userInitiated).asyncAfter(deadline: .now() + 0.1) {
            promise(.success(value))
        }
    }.eraseToAnyPublisher()
}

var count = 3

let s = PassthroughSubject<Void, Never>()

var cancellables = Set<AnyCancellable>([])

// This reproduces the problem. Switching buffer and combineLatest fix the problem…

s
    .print()
    .buffer(size: Int.max, prefetch: .keepFull, whenFull: .dropNewest)
    .combineLatest(Just("a"))
    .filter { _ in count > 0 }
    .flatMap(maxPublishers: .max(1)) { _, a in process("\(count)") }
    .sink {
        print($0)
        count -= 1
        s.send()
    }
    .store(in: &cancellables)

s.send()

Thread.sleep(forTimeInterval: 3)

count = 1
s.send()

Switching combine and buffer fixes the problem.

Kamchatka
  • 3,597
  • 4
  • 38
  • 69
  • I can't repro this - removing some of the UIKit stuff and substituting button tap with just `add`, outputs all the added values. Also, if feels a bit overly convoluted. Why do you send value on remove? Why do you have `.combineLatest` with `Just(true)`? A subject + buffer + flatMap is all you'd need to implement a queue, I think. I don't think even an array is needed there. – New Dev Feb 02 '21 at 00:03
  • As I mentioned, "I'm using a combineLatest operator to only trigger the pipeline under some conditions. For simplicity, I'm using a Just(true) publisher here." I'm not implementing a queue here, the queue is given to me, but I'm using Combine to implement the continuous processing of elements from the queue. – Kamchatka Feb 02 '21 at 08:29
  • Couldn't you just have the publisher publish the value at the head of the queue and use `receive(on:)` with a serial dispatch queue to ensure you only process one element at a time. – Paulw11 Feb 02 '21 at 09:07
  • That would work if the operation in the flatmap (`process()`) was synchronous, but here it's asynchronous and can be dispatched anywhere (not on the serial queue), so the serial queue will just have the effect to serialize the _calls_ to `process()` – Kamchatka Feb 02 '21 at 09:32
  • My initial example was very convoluted, so I have simplified it and opened a new question to avoid making comments here obsolete: https://stackoverflow.com/questions/66007062/combine-pipeline-not-receiving-values – Kamchatka Feb 02 '21 at 09:40
  • @Kamchatka, I ran your latest example as-is in Playground and it outputs 3 values in sink – New Dev Feb 02 '21 at 14:27
  • It never outputs the value corresponding to the latest send(). But please see the new question here: https://stackoverflow.com/questions/66007062/combine-pipeline-not-receiving-values which is more clear. – Kamchatka Feb 02 '21 at 16:36

2 Answers2

0

I am not sure why the pipeline is blocked, but there is no reason to publish when the queue is empty. Fixing this resolved the problem for me.

func removeFirst() {
    guard !strings.isEmpty else {
        return
    }
    strings.removeFirst()
    if !self.strings.isEmpty {
        self.changePublisher.send(self.strings.first)
    }
}
Paulw11
  • 108,386
  • 14
  • 159
  • 186
  • Thanks. That's indeed a good suggestion, but I think it's similar to what happens when I exchange `buffer` and `combine`, which solves the problem. I have edited my original post with a simpler example, without the queue, that also demonstrates the same problem. – Kamchatka Feb 02 '21 at 08:32
0

Just tried your example. It works as expected if buffer is placed before flatMap. And update removeFirst according to Paulw11's answer below

queue.changePublisher
            .print("Change")
            // NOTE: If I remove this combineLatest (and the filter below, to make it compile), I don't have the issue anymore.
            .combineLatest(Just(true))
            .print("Combine")
            .filter { _, enabled in return enabled }
            .print("Filter")
            .compactMap { _ in
                self.queue.peek()
            }
            .print("Compact")
            // maxPublishers lets us process one page at a time
            .buffer(size: Int.max, prefetch: .keepFull, whenFull: .dropNewest)
            .print("Buffer")
            .flatMap(maxPublishers: .max(1)) { reference in
                return self.process(reference)
            }
            .sink { reference in
                print("Sink for \(reference)")

                // Remove the processed item from the queue. This will also trigger the queue's changePublisher,
                // which re-run this pipeline in case
                self.queue.removeFirst()
                print("COUNT: " + self.queue.strings.count.description)
            }
            .store(in: &cancellables)
kefir
  • 21
  • 4