From 9342c75f1d9bbbcbdd40b5c1b8eb48cb0bb7df26 Mon Sep 17 00:00:00 2001 From: Nikolay Kasyanov Date: Fri, 24 May 2024 17:32:07 +0200 Subject: [PATCH 1/4] Add failing test --- Tests/RxSwiftTests/Completable+AndThen.swift | 35 ++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/Tests/RxSwiftTests/Completable+AndThen.swift b/Tests/RxSwiftTests/Completable+AndThen.swift index 2ea8c26b2..649210269 100644 --- a/Tests/RxSwiftTests/Completable+AndThen.swift +++ b/Tests/RxSwiftTests/Completable+AndThen.swift @@ -120,6 +120,38 @@ extension CompletableAndThenTest { ]) } + func testCompletable_FirstCompletableNotRetainedBeyondCompletion() { + let scheduler = TestScheduler(initialClock: 0) + + let x: TestableObservable = scheduler.createHotObservable([ + .completed(210), + ]) + + var object = Optional.some(TestObject()) + + var completable = x.asCompletable() + .do(onCompleted: { [object] in + _ = object + }) + + let disposable = completable + .andThen(.never()) + .subscribe() + + defer { + disposable.dispose() + } + + // completable has completed by now + scheduler.advanceTo(300) + + weak var weakObject = object + object = nil + completable = .never() + + XCTAssertNil(weakObject) + } + #if TRACE_RESOURCES func testAndThenCompletableReleasesResourcesOnComplete() { _ = Completable.empty().andThen(Completable.empty()).subscribe() @@ -575,3 +607,6 @@ extension CompletableAndThenTest { } #endif } + +private class TestObject { +} From c645e458ffc344552c0aebf4d02bd599bca01a07 Mon Sep 17 00:00:00 2001 From: Nikolay Kasyanov Date: Fri, 24 May 2024 17:34:23 +0200 Subject: [PATCH 2/4] Don't retain receiver of Completable.andThen beyond its completion --- .../Completable+AndThen.swift | 16 ++++----- Tests/RxSwiftTests/Completable+AndThen.swift | 34 ++++++++++++++++++- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/RxSwift/Traits/PrimitiveSequence/Completable+AndThen.swift b/RxSwift/Traits/PrimitiveSequence/Completable+AndThen.swift index f63c3f17f..dd1dc9723 100644 --- a/RxSwift/Traits/PrimitiveSequence/Completable+AndThen.swift +++ b/RxSwift/Traits/PrimitiveSequence/Completable+AndThen.swift @@ -70,8 +70,8 @@ final private class ConcatCompletable: Producer { } override func run(_ observer: Observer, cancel: Cancelable) -> (sink: Disposable, subscription: Disposable) where Observer.Element == Element { - let sink = ConcatCompletableSink(parent: self, observer: observer, cancel: cancel) - let subscription = sink.run() + let sink = ConcatCompletableSink(second: second, observer: observer, cancel: cancel) + let subscription = sink.run(completable: completable) return (sink: sink, subscription: subscription) } } @@ -82,11 +82,11 @@ final private class ConcatCompletableSink typealias Element = Never typealias Parent = ConcatCompletable - private let parent: Parent + private let second: Observable private let subscription = SerialDisposable() - init(parent: Parent, observer: Observer, cancel: Cancelable) { - self.parent = parent + init(second: Observable, observer: Observer, cancel: Cancelable) { + self.second = second super.init(observer: observer, cancel: cancel) } @@ -99,14 +99,14 @@ final private class ConcatCompletableSink break case .completed: let otherSink = ConcatCompletableSinkOther(parent: self) - self.subscription.disposable = self.parent.second.subscribe(otherSink) + self.subscription.disposable = self.second.subscribe(otherSink) } } - func run() -> Disposable { + func run(completable: Observable) -> Disposable { let subscription = SingleAssignmentDisposable() self.subscription.disposable = subscription - subscription.setDisposable(self.parent.completable.subscribe(self)) + subscription.setDisposable(completable.subscribe(self)) return self.subscription } } diff --git a/Tests/RxSwiftTests/Completable+AndThen.swift b/Tests/RxSwiftTests/Completable+AndThen.swift index 649210269..c9276918d 100644 --- a/Tests/RxSwiftTests/Completable+AndThen.swift +++ b/Tests/RxSwiftTests/Completable+AndThen.swift @@ -125,7 +125,39 @@ extension CompletableAndThenTest { let x: TestableObservable = scheduler.createHotObservable([ .completed(210), - ]) + ]) + + var object = Optional.some(TestObject()) + + var completable = x.asCompletable() + .do(onCompleted: { [object] in + _ = object + }) + + let disposable = completable + .andThen(.never()) + .subscribe() + + defer { + disposable.dispose() + } + + // completable has completed by now + scheduler.advanceTo(300) + + weak var weakObject = object + object = nil + completable = .never() + + XCTAssertNil(weakObject) + } + + func testCompletable_FirstCompletableNotRetainedBeyondFailure() { + let scheduler = TestScheduler(initialClock: 0) + + let x: TestableObservable = scheduler.createHotObservable([ + .error(210, TestError.dummyError), + ]) var object = Optional.some(TestObject()) From e39bcb965ecd29812520c0df5de8094e74155c9c Mon Sep 17 00:00:00 2001 From: Nikolay Kasyanov Date: Fri, 24 May 2024 17:38:51 +0200 Subject: [PATCH 3/4] Fix comment --- Tests/RxSwiftTests/Completable+AndThen.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/RxSwiftTests/Completable+AndThen.swift b/Tests/RxSwiftTests/Completable+AndThen.swift index c9276918d..5ae78cf02 100644 --- a/Tests/RxSwiftTests/Completable+AndThen.swift +++ b/Tests/RxSwiftTests/Completable+AndThen.swift @@ -174,7 +174,7 @@ extension CompletableAndThenTest { disposable.dispose() } - // completable has completed by now + // completable has terminated with error by now scheduler.advanceTo(300) weak var weakObject = object From 426a9c6b7a7e69cf06da93c756876ab1e0eb0bb9 Mon Sep 17 00:00:00 2001 From: Shai Mishali Date: Fri, 4 Oct 2024 20:51:02 +0300 Subject: [PATCH 4/4] Tests script --- Sources/AllTestz/main.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/AllTestz/main.swift b/Sources/AllTestz/main.swift index 4a0dc0a4b..ab665d5fd 100644 --- a/Sources/AllTestz/main.swift +++ b/Sources/AllTestz/main.swift @@ -105,6 +105,8 @@ final class CompletableAndThenTest_ : CompletableAndThenTest, RxTestCase { ("testCompletableCompleted_CompletableCompleted", CompletableAndThenTest.testCompletableCompleted_CompletableCompleted), ("testCompletableError_CompletableCompleted", CompletableAndThenTest.testCompletableError_CompletableCompleted), ("testCompletableCompleted_CompletableError", CompletableAndThenTest.testCompletableCompleted_CompletableError), + ("testCompletable_FirstCompletableNotRetainedBeyondCompletion", CompletableAndThenTest.testCompletable_FirstCompletableNotRetainedBeyondCompletion), + ("testCompletable_FirstCompletableNotRetainedBeyondFailure", CompletableAndThenTest.testCompletable_FirstCompletableNotRetainedBeyondFailure), ("testCompletableEmpty_SingleCompleted", CompletableAndThenTest.testCompletableEmpty_SingleCompleted), ("testCompletableCompleted_SingleNormal", CompletableAndThenTest.testCompletableCompleted_SingleNormal), ("testCompletableError_SingleNormal", CompletableAndThenTest.testCompletableError_SingleNormal),