Starting a SignalProducer with debounce twice doesn't deliver values to the second observer

[gpambrozio]

This is related to #718.

I just found something similar on my project and I believe debounce to be broken when you start a producer twice. With latest master, I added the following tests to SignalProducerSpec.swift and they both fail (just the second expectation of each it, not the first):

describe("debounce") {
	context("property - starting twice") {
		it("should deliver the same values") {
			let p1 = MutableProperty(1)
			let p2 = MutableProperty(2)

			let scheduler = TestScheduler()
			let combined = Property.combineLatest(p1, p2)
			let prod = combined
				.producer
				.skipRepeats { $0 == $1 }
				.map { $0 + $1 }
				.debounce(0.1, on: scheduler)

			var v1 = [Int]()
			var v2 = [Int]()
			prod.startWithValues { (v) in
				v1.append(v)
			}
			prod.startWithValues { (v) in
				v2.append(v)
			}

			scheduler.advance(by: .milliseconds(200))
			p1.value = 3
			scheduler.advance(by: .milliseconds(200))
			p2.value = 4
			scheduler.advance(by: .milliseconds(200))
			p2.value = 4
			scheduler.advance(by: .milliseconds(200))

			expect(v1).to(equal([3, 5, 7]))
			expect(v2).to(equal([3, 5, 7]))
		}
	}

	context("pipe - starting twice") {
		it("should deliver the same values") {
			let p1 = SignalProducer<Int, Never>.pipe()
			let p2 = SignalProducer<Int, Never>.pipe()

			let scheduler = TestScheduler()
			let combined = SignalProducer.combineLatest(p1.0, p2.0)
			let prod = combined
				.skipRepeats { $0 == $1 }
				.map { $0 + $1 }
				.debounce(0.1, on: scheduler)

			var v1 = [Int]()
			var v2 = [Int]()
			prod.startWithValues { (v) in
				v1.append(v)
			}
			prod.startWithValues { (v) in
				v2.append(v)
			}

			p1.1.send(value: 1)
			p2.1.send(value: 2)
			scheduler.advance(by: .milliseconds(200))
			p1.1.send(value: 3)
			scheduler.advance(by: .milliseconds(200))
			p2.1.send(value: 4)
			scheduler.advance(by: .milliseconds(200))
			p2.1.send(value: 4)
			scheduler.advance(by: .milliseconds(200))

			expect(v1).to(equal([3, 5, 7]))
			expect(v2).to(equal([3, 5, 7]))

		}
	}
}

If you remove the debounce from the chain they succeed.

I understand why changing it to a Signal would fix this but shouldn't this work for this case as well?

I noticed this on one of my projects after I updated ReactiveSwift from 3.1 to 6.2. On 3.1 this was working as I'd expect but on 6.1 my feature broke because of this.

I'm afraid it might be because of #605 as this is the only change regarding debounce on the changelog but I have not confirmed that yet.

I'm investigating further and if I find something I'll submit a PR but want to have the issue to track and have some more expert eyes on it.

[gpambrozio]

I reverted (most of) #605 and the tests both fail. Not the solution but it shows that this is what broke the behavior and (and least imho) shows it's broken now.

[andersio]

Argh, state is shared across produced signals. That's why it is broken when it is started more than once. 🙈

[andersio]

Moving https://github.com/ReactiveCocoa/ReactiveSwift/blob/master/Sources/Event.swift#L873 into the outer closure (not the inner one, which is the event sink) should resolve the issue.

[gpambrozio]

You got it! I'll submit a PR Monday with the fix and added tests. Thanks for the quick help.

[gpambrozio]

Should be fixed as part of #772