Merge pull request #463 from ReactiveCocoa/auto-interrupt

Send `interrupted` when an input observer deinitializes.

Author: mdiep

CHANGELOG.md

@@ -1,6 +1,8 @@
 # master
 *Please add new entries at the top.*
 
+1. If the input observer of a `Signal` deinitializes while the `Signal` has not yet terminated, an `interrupted` event would now be automatically sent. (#463, kudos to @andersio)
+
 1. `ValidationResult` and `ValidatorOutput` have been renamed to `ValidatingProperty.Result` and `ValidatingProperty.Decision`, respectively. (#443)
 
 1. Mitigated a race condition related to ARC in the `Signal` internal. (#456, kudos to @andersio)

Sources/Observer.swift

@@ -15,13 +15,29 @@ extension Signal {
 		/// An action that will be performed upon arrival of the event.
 		public let action: Action
 
+		/// Whether the observer should send an `interrupted` event as it deinitializes.
+		private let interruptsOnDeinit: Bool
+
+		/// An initializer that accepts a closure accepting an event for the
+		/// observer.
+		///
+		/// - parameters:
+		///   - action: A closure to lift over received event.
+		///   - interruptsOnDeinit: `true` if the observer should send an `interrupted`
+		///                         event as it deinitializes. `false` otherwise.
+		internal init(action: @escaping Action, interruptsOnDeinit: Bool) {
+			self.action = action
+			self.interruptsOnDeinit = interruptsOnDeinit
+		}
+
 		/// An initializer that accepts a closure accepting an event for the 
 		/// observer.
 		///
 		/// - parameters:
 		///   - action: A closure to lift over received event.
 		public init(_ action: @escaping Action) {
 			self.action = action
+			self.interruptsOnDeinit = false
 		}
 
 		/// An initializer that accepts closures for different event types.
@@ -68,6 +84,15 @@ extension Signal {
 			}
 		}
 
+		deinit {
+			if interruptsOnDeinit {
+				// Since `Signal` would ensure that only one terminal event would ever be
+				// sent for any given `Signal`, we do not need to assert any condition
+				// here.
+				action(.interrupted)
+			}
+		}
+
 		/// Puts a `value` event into `self`.
 		///
 		/// - parameters:

Sources/Signal.swift

@@ -70,7 +70,7 @@ public final class Signal<Value, Error: Swift.Error> {
 			disposable = SerialDisposable()
 
 			// The generator observer retains the `Signal` core.
-			disposable.inner = generator(Observer(self.send))
+			disposable.inner = generator(Observer(action: self.send, interruptsOnDeinit: true))
 		}
 
 		private func send(_ event: Event) {
@@ -376,7 +376,11 @@ public final class Signal<Value, Error: Swift.Error> {
 extension Signal {
 	/// A Signal that never sends any events to its observers.
 	public static var never: Signal {
-		return self.init { _ in nil }
+		return self.init { observer in
+			// If `observer` deinitializes, the `Signal` would interrupt which is
+			// undesirable for `Signal.never`.
+			return AnyDisposable { _ = observer }
+		}
 	}
 
 	/// A Signal that completes immediately without emitting any value.

Tests/ReactiveSwiftTests/PropertySpec.swift

@@ -1680,17 +1680,19 @@ class PropertySpec: QuickSpec {
 				}
 
 				it("should tear down the binding when the property deallocates") {
-					let (signal, _) = Signal<String, NoError>.pipe()
+					let (signal, observer) = Signal<String, NoError>.pipe()
 					let signalProducer = SignalProducer(signal)
 
 					var mutableProperty: MutableProperty<String>? = MutableProperty(initialPropertyValue)
 
-					var isDisposed = false
-					mutableProperty! <~ signalProducer.on(disposed: { isDisposed = true })
-					expect(isDisposed) == false
+					withExtendedLifetime(observer) {
+						var isDisposed = false
+						mutableProperty! <~ signalProducer.on(disposed: { isDisposed = true })
+						expect(isDisposed) == false
 
-					mutableProperty = nil
-					expect(isDisposed) == true
+						mutableProperty = nil
+						expect(isDisposed) == true
+					}
 				}
 			}
 

Tests/ReactiveSwiftTests/SignalLifetimeSpec.swift

@@ -22,6 +22,44 @@ class SignalLifetimeSpec: QuickSpec {
 				testScheduler = TestScheduler()
 			}
 
+			it("should automatically interrupt if the input observer is not retained") {
+				let disposable = AnyDisposable()
+				var outerSignal: Signal<Never, NoError>!
+
+				func scope() {
+					let (signal, observer) = Signal<Never, NoError>.pipe(disposable: disposable)
+					outerSignal = signal
+
+					expect(disposable.isDisposed) == false
+				}
+
+				scope()
+				expect(disposable.isDisposed) == true
+			}
+
+
+			it("should automatically interrupt if the input observer is not retained, even if there are still one or more active observer") {
+				let disposable = AnyDisposable()
+				var isInterrupted = false
+				var outerSignal: Signal<Never, NoError>!
+
+				func scope() {
+					let (signal, observer) = Signal<Never, NoError>.pipe(disposable: disposable)
+					outerSignal = signal
+
+					signal.observeInterrupted {
+						isInterrupted = true
+					}
+
+					expect(isInterrupted) == false
+					expect(disposable.isDisposed) == false
+				}
+
+				scope()
+				expect(isInterrupted) == true
+				expect(disposable.isDisposed) == true
+			}
+
 			it("should be disposed of if it does not have any observers") {
 				var isDisposed = false
 
@@ -35,20 +73,29 @@ class SignalLifetimeSpec: QuickSpec {
 
 			it("should be disposed of if no one retains it") {
 				var isDisposed = false
-				var signal: Signal<AnyObject, NoError>? = Signal { _ in nil }.on(disposed: { isDisposed = true })
+				var observer: Signal<AnyObject, NoError>.Observer!
+				var signal: Signal<AnyObject, NoError>? = Signal
+					.init { innerObserver in
+						observer = innerObserver
+						return nil
+					}
+					.on(disposed: { isDisposed = true })
+
 				weak var weakSignal = signal
 
-				expect(weakSignal).toNot(beNil())
-				expect(isDisposed) == false
+				withExtendedLifetime(observer) {
+					expect(weakSignal).toNot(beNil())
+					expect(isDisposed) == false
 
-				var reference = signal
-				signal = nil
-				expect(weakSignal).toNot(beNil())
-				expect(isDisposed) == false
+					var reference = signal
+					signal = nil
+					expect(weakSignal).toNot(beNil())
+					expect(isDisposed) == false
 
-				reference = nil
-				expect(weakSignal).to(beNil())
-				expect(isDisposed) == true
+					reference = nil
+					expect(weakSignal).to(beNil())
+					expect(isDisposed) == true
+				}
 			}
 
 			it("should be disposed of when the signal shell has deinitialized with no active observer regardless of whether the generator observer is retained or not") {

Tests/ReactiveSwiftTests/SignalProducerLiftingSpec.swift

@@ -1430,12 +1430,14 @@ class SignalProducerLiftingSpec: QuickSpec {
 				}
 
 				var sampledProducer: SignalProducer<Payload, NoError>!
+				var samplerObserver: Signal<(), NoError>.Observer!
 				var observer: Signal<Payload, NoError>.Observer!
 
 				beforeEach {
 					let (producer, incomingObserver) = SignalProducer<Payload, NoError>.pipe()
-					let (sampler, _) = Signal<(), NoError>.pipe()
+					let (sampler, _samplerObserver) = Signal<(), NoError>.pipe()
 					sampledProducer = producer.sample(on: sampler)
+					samplerObserver = _samplerObserver
 					observer = incomingObserver
 				}
 

Tests/ReactiveSwiftTests/SignalProducerSpec.swift

@@ -1049,18 +1049,20 @@ class SignalProducerSpec: QuickSpec {
 			}
 
 			it("should attach event handlers for disposal") {
-				let (baseProducer, _) = SignalProducer<Int, TestError>.pipe()
+				let (baseProducer, observer) = SignalProducer<Int, TestError>.pipe()
 
-				var disposed: Bool = false
+				withExtendedLifetime(observer) {
+					var disposed: Bool = false
 
-				let producer = baseProducer
-					.on(disposed: { disposed = true })
+					let producer = baseProducer
+						.on(disposed: { disposed = true })
 
-				let disposable = producer.start()
+					let disposable = producer.start()
 
-				expect(disposed) == false
-				disposable.dispose()
-				expect(disposed) == true
+					expect(disposed) == false
+					disposable.dispose()
+					expect(disposed) == true
+				}
 			}
 
 			it("should invoke the `started` action of the inner producer first") {
@@ -1323,12 +1325,15 @@ class SignalProducerSpec: QuickSpec {
 					var completeB: (() -> Void)!
 					var sendB: (() -> Void)!
 
+					var outerObserver: Signal<SignalProducer<Int, NoError>, NoError>.Observer!
 					var outerCompleted = false
 
 					var recv = [Int]()
 
 					beforeEach {
-						let (outerProducer, outerObserver) = SignalProducer<SignalProducer<Int, NoError>, NoError>.pipe()
+						let (outerProducer, _outerObserver) = SignalProducer<SignalProducer<Int, NoError>, NoError>.pipe()
+						outerObserver = _outerObserver
+
 						let (producerA, observerA) = SignalProducer<Int, NoError>.pipe()
 						let (producerB, observerB) = SignalProducer<Int, NoError>.pipe()
 
@@ -1358,6 +1363,15 @@ class SignalProducerSpec: QuickSpec {
 						outerObserver.sendCompleted()
 					}
 
+					afterEach {
+						(completeA, completeB) = (nil, nil)
+						(sendA, sendB) = (nil, nil)
+						outerObserver = nil
+
+						outerCompleted = false
+						recv = []
+					}
+
 					it("should forward values from any inner signals") {
 						sendA()
 						sendA()

Tests/ReactiveSwiftTests/SignalSpec.swift

@@ -260,19 +260,21 @@ class SignalSpec: QuickSpec {
 
 			context("memory") {
 				it("should not crash allocating memory with a few observers") {
-					let (signal, _) = Signal<Int, NoError>.pipe()
+					let (signal, observer) = Signal<Int, NoError>.pipe()
 
 					#if os(Linux)
 						func autoreleasepool(invoking code: () -> Void) {
 							code()
 						}
 					#endif
 
-					for _ in 0..<50 {
-						autoreleasepool {
-							let disposable = signal.observe { _ in }
+					withExtendedLifetime(observer) {
+						for _ in 0..<50 {
+							autoreleasepool {
+								let disposable = signal.observe { _ in }
 
-							disposable!.dispose()
+								disposable!.dispose()
+							}
 						}
 					}
 				}