ReactiveX / RxSwift

Reactive Programming in Swift
MIT License
24.37k stars 4.17k forks source link

Factory closure lifetime differs between .create and .deferred, subscriptions to the latter keep its factory closure alive #2533

Closed nikolaykasyanov closed 1 year ago

nikolaykasyanov commented 1 year ago

⚠️ it's an issue report in the form of a pull request, to have a self-contained runnable examples.

Short description of the issue:

Each subscription to Observable.deferred captures its factory closure, unlike Observable.create.

Expected outcome:

Intuitively I would expect Observable.deferred to not keep its factory closure alive longer than required in subscriptions, but that's probably not the best argument.

Alternatively, I'd expect this behavior documented because I recognize that changing it at this point might be a very breaking change.

What actually happens:

.deferred is implemented in such a way each subscription to it retains its factory closure: https://github.com/ReactiveX/RxSwift/blob/2ff073fc1b9c600ac9b54734aff3348110437c19/RxSwift/Observables/Deferred.swift#L71

Self contained code example that reproduces the issue:

See the test cases in this PR. testDeferredFactoryClosureLifetime fails while testObservableFactoryClosureLifetime passes.

RxSwift/RxCocoa/RxBlocking/RxTest version/commit

6.6.0

Platform/Environment

How easy is to reproduce? (chances of successful reproduce after running the self contained code)

Xcode version:

  Version 14.3.1 (14E300c)

:warning: Fields below are optional for general issues or in case those questions aren't related to your issue, but filling them out will increase the chances of getting your issue resolved. :warning:

Installation method:

I have multiple versions of Xcode installed: (so we can know if this is a potential cause of your issue)

Level of RxSwift knowledge: (this is so we can understand your level of knowledge and formulate the response in an appropriate manner)

danielt1263 commented 1 year ago

I made a PR with the fix. #2534

nikolaykasyanov commented 1 year ago

Closing in favour of #2534.