Retain cycle in bind(to:input:)
Like the title says there is a retain cycle in the various bind(to:input:) functions. The following test fails:
it("does not create a retain cycle") {
var subject: UIBarButtonItem? = UIBarButtonItem(barButtonSystemItem: .save, target: nil, action: nil)
let action = Action<String, String>(workFactory: { _ in
return .empty()
})
subject?.rx.bind(to: action, input: "Hi there!")
weak var subjectWeakReference = subject
subject = nil
expect(subjectWeakReference).to(beNil()) // fails!
}
The retain cycle is due to passing base to inputTransform in map. This can be fixed by changing the various bind methods to the following:
self.tap // or controlEvent
.withUnretained(self.base) // no longer retaining base!
.map(\.0)
.map(inputTransform)
.bind(to: action.inputs)
.disposed(by: self.base.actionDisposeBag)
With the change above all tests plus the one I added to check for a retain cycle pass. I'd be happy to write up a pull request to make these changes if you agree they need to be made.
P.s. Thanks for the great framework!
Hi @nflahavan! Thank you for the detailed issue – the unit test is very helpful. Definitely a reference cycle here would be a bug. I would be happy to review a PR, thank you again.
https://github.com/RxSwiftCommunity/Action/pull/228 is available for your review whenever you get a chance!
Hey @ashfurrow @nflahavan the issue is still not fixed. I'm using the Action version from the master, and still I can detect memory leaks
This simple code generates memory leak
let forgotPasswordAction = CocoaAction {
return .create { [weak self] (observer) -> Disposable in
guard let self = self else { return Disposables.create() }
self.delegate?.didTapForgotPasswordButton()
observer.onCompleted()
return Disposables.create()
}
}
forgotPasswordButton.rx.action = forgotPasswordAction
#235 It's very similar but for button.rx.action case
Hi, thanks for following up and opening #235. I honestly don't use Action or RxSwift day-to-day anymore, so I don't have a lot of time to devote to fixing bugs like this. I'd be happy to review a pull request, though.
I also just noticed that #228 was merged but hasn't been released yet – let's wait until this further retain cycle is fixed and then release a version 4.3.1 👍