-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't retain receiver of Completable.andThen beyond its completion #2604
Don't retain receiver of Completable.andThen beyond its completion #2604
Conversation
private let subscription = SerialDisposable() | ||
|
||
init(parent: Parent, observer: Observer, cancel: Cancelable) { | ||
self.parent = parent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure whether not retaining parent
is the right thing to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels very much like the change I made for the deferred
code (214f7ff). If the Completable has emitted a completed event, there is no reason to keep it around.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danielt1263 indeed! Given its similarity to the deferred fix, I wonder if this PR can be considered for a minor release? cc @freak4pc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If tests pass this LGTM!
Otherwise the creation closure and operator closures involved in the Completable (and hence all objects captured by them) are retained until the outer stream completes or terminates with error.