Fixed a bug in on which caused the signal to be disposed of too early.

[andersio]

Fixes #77.

The root cause

 		return SignalProducer { observer, compositeDisposable in
  			// ...
  			self.startWithSignal { signal, disposable in
  				compositeDisposable += disposable
 				compositeDisposable += signal.on(...).observe(observer)

The outer input observer was detached by the outer interruption, and this causes the started signal to dispose of itself (no observer + unreachable) under the new lifetime semantics.

Moreover, since CompositeDisposable disposes its inner disposables in the reversed insertion order, the inner producer is interrupted after the said detachment and self-disposal happened.

So in the end, both together caused the injected side effect via on capturing no interrupted event.

The fix

Generally speaking, the disposable of the outer observer to the produced inner signal should not be added to the outer producer's disposable. The termination is already propagated via interrupted.

Note

We might need to review the rest of the operators.

[inamiy]

Thanks for catch!
It seems reversing compositeDisposable += ... also works well (so that self producer's signal is safely interrupted before signal.on(...).observe(observer)'s disposable is being disposed.

        return SignalProducer { observer, compositeDisposable in
            // ...
            self.startWithSignal { signal, disposable in
                compositeDisposable += signal.on(...).observe(observer)
                compositeDisposable += disposable

[andersio]

Disposing the cancel disposable of a produced signal is guaranteed to have interrupted emitted, should the signal not have been terminated.

In other words, adding the observer disposable to the outer disposable is redundant - there is no point to detach from a known-to-be terminated signal.

Moreover, the iteration order is an implementation detail of Bag that should not be depended on unless absolutely necessary, as Bag by contract is an unordered collection.

[andersio]

@ikesyo Would you mind to have a look at why Linux PM test fails? It says toEventually is missing, but it seems the package config of Nimble is fine.

[mdiep]

Looks good to me apart from the linux failures.

[andersio]

Linux test error would be fixed by #81.

[inamiy]

@andersio I see. Thanks for explanation. LGTM!

[ikesyo]

@andersio Carthage/Checkouts/Nimble submodule would not be related to SwiftPM integration unfortunately. I will take a look at the failure.

[andersio]

@ikesyo I found that 5.0.0 wrapped the async matchers in something like #if _runtime(objc), which seems to be the cause. Had been removed in 5.1.1 though.

[ikesyo]

@andersio That is the reason I checked out a specific commit in .travis.yml: https://github.com/ReactiveCocoa/ReactiveSwift/blob/master/.travis.yml#L85 (#47). See #68 too (I'm waiting a new release of Quick).