Skip to content

Commit

Permalink
Amb: forward cancel to the inner publishers
Browse files Browse the repository at this point in the history
We noticed that cancelling an Amb publisher does not cancel any of the inner publishers.
It looks like nil-ing firstSink / secondSink should do the job, but it does not work. Adding
explicit cancel fixes the issue, but I’m not sure if this is the right solution.
  • Loading branch information
melle committed Sep 6, 2022
1 parent aaf1a09 commit 892b818
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 0 deletions.
2 changes: 2 additions & 0 deletions Sources/Operators/Amb.swift
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,9 @@ private extension Publishers.Amb {
}

func cancel() {
firstSink?.cancelUpstream()
firstSink = nil
secondSink?.cancelUpstream()
secondSink = nil
}
}
Expand Down
31 changes: 31 additions & 0 deletions Tests/AmbTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,37 @@ class AmbTests: XCTestCase {
XCTAssertEqual(completion, .finished)
}

func testAmbCancelPreSubscription() {
enum CancelError: Swift.Error {
case cancelled
}
var ambPublisher: AnyCancellable?

var firstCompletion: Subscribers.Completion<CancelError>?
let subject1 = PassthroughSubject<Int, CancelError>()
let subject1Publisher = subject1
.handleEvents(receiveCancel: {
firstCompletion = .failure(CancelError.cancelled)
})
.eraseToAnyPublisher()

var secondCompletion: Subscribers.Completion<CancelError>?
let subject2 = PassthroughSubject<Int, CancelError>()
let subject2Publisher = subject2
.handleEvents(receiveCancel: {
secondCompletion = .failure(CancelError.cancelled)
})
.eraseToAnyPublisher()

ambPublisher = Publishers.Amb(first: subject1Publisher, second: subject2Publisher)
.sink(receiveCompletion: { _ in },
receiveValue: { _ in })
ambPublisher?.cancel()

XCTAssertEqual(firstCompletion, .failure(CancelError.cancelled))
XCTAssertEqual(secondCompletion, .failure(CancelError.cancelled))
}

func testAmbLimitedPreDemand() {
let subject1 = PassthroughSubject<Int, Never>()
let subject2 = PassthroughSubject<Int, Never>()
Expand Down

0 comments on commit 892b818

Please sign in to comment.