From 214f7ffced20dc780bf38f192a26a890c1b9045e Mon Sep 17 00:00:00 2001 From: Daniel Tartaglia Date: Sat, 28 Oct 2023 16:17:17 -0400 Subject: [PATCH] Fix: Factory closure lifetime differs between .create and .deferred, subscriptions to the latter keep its factory closure alive #2533 (#2534) --- RxSwift/Observables/Deferred.swift | 20 +++---- Tests/RxSwiftTests/Observable+Tests.swift | 72 +++++++++++++++++++++++ 2 files changed, 81 insertions(+), 11 deletions(-) diff --git a/RxSwift/Observables/Deferred.swift b/RxSwift/Observables/Deferred.swift index b81cb1669..51e64d1c0 100644 --- a/RxSwift/Observables/Deferred.swift +++ b/RxSwift/Observables/Deferred.swift @@ -22,18 +22,16 @@ extension ObservableType { } final private class DeferredSink: Sink, ObserverType where Source.Element == Observer.Element { - typealias Element = Observer.Element - - private let observableFactory: () throws -> Source - - init(observableFactory: @escaping () throws -> Source, observer: Observer, cancel: Cancelable) { - self.observableFactory = observableFactory + typealias Element = Observer.Element + typealias Parent = Deferred + + override init(observer: Observer, cancel: Cancelable) { super.init(observer: observer, cancel: cancel) } - func run() -> Disposable { + func run(_ parent: Parent) -> Disposable { do { - let result = try self.observableFactory() + let result = try parent.observableFactory() return result.subscribe(self) } catch let e { @@ -60,7 +58,7 @@ final private class DeferredSink final private class Deferred: Producer { typealias Factory = () throws -> Source - private let observableFactory : Factory + let observableFactory : Factory init(observableFactory: @escaping Factory) { self.observableFactory = observableFactory @@ -68,8 +66,8 @@ final private class Deferred: Producer { override func run(_ observer: Observer, cancel: Cancelable) -> (sink: Disposable, subscription: Disposable) where Observer.Element == Source.Element { - let sink = DeferredSink(observableFactory: self.observableFactory, observer: observer, cancel: cancel) - let subscription = sink.run() + let sink = DeferredSink(observer: observer, cancel: cancel) + let subscription = sink.run(self) return (sink: sink, subscription: subscription) } } diff --git a/Tests/RxSwiftTests/Observable+Tests.swift b/Tests/RxSwiftTests/Observable+Tests.swift index f5d0c0783..efca33e12 100644 --- a/Tests/RxSwiftTests/Observable+Tests.swift +++ b/Tests/RxSwiftTests/Observable+Tests.swift @@ -228,6 +228,78 @@ extension ObservableTest { } } +// MARK: - Deferred +extension ObservableTest { + func testDeferredFactoryClosureLifetime() { + class Foo { + let expectation: XCTestExpectation + + init(expectation: XCTestExpectation) { + self.expectation = expectation + } + + func bar() -> Observable { + Observable + .deferred { + self.expectation.fulfill() + return .never() + } + } + } + + let factoryClosureInvoked = expectation(description: "Factory closure has been invoked") + var foo: Foo? = Foo(expectation: factoryClosureInvoked) + weak var initialFoo = foo + + let disposable = foo?.bar().subscribe() + + wait(for: [factoryClosureInvoked]) + + // reset foo to let the initial instance deallocate + foo = nil + + // we know that the factory closure has already been executed, + // and the foo reference has been nilled, so there should be nothing + // keeping the object alive + XCTAssertNil(initialFoo) + + disposable?.dispose() + } + + func testObservableFactoryClosureLifetime() { + class Foo { + let expectation: XCTestExpectation + + init(expectation: XCTestExpectation) { + self.expectation = expectation + } + + func bar() -> Observable { + Observable + .create { _ in + self.expectation.fulfill() + return Disposables.create() + } + } + } + + let factoryClosureInvoked = expectation(description: "Factory closure has been invoked") + var foo: Foo? = Foo(expectation: factoryClosureInvoked) + weak var initialFoo = foo + + let disposable = foo?.bar().subscribe() + + wait(for: [factoryClosureInvoked]) + + // reset foo to let the initial instance deallocate + foo = nil + + XCTAssertNil(initialFoo) + + disposable?.dispose() + } +} + private class TestObject: NSObject { var id = UUID() }