Are bound Tweaks leaking the observation blocks?

[mac-cain13]

We're using SwiftTweaks in a project and while using bind for a few Tweaks I was wondering when those observer closures are released. I skimmed through the source code and it appears to me that the closure given to bind is saved in a dictionary where it is strongly references forever.

If I'm not mistaken these bindings dictionaries are never cleaned up neither there is any API to unregister the binding. So everytime my ViewController is started I bind a new closure that will be around forever, this looks like a memory leak to me.

Is there anything I'm missing that resolved this memory issue? Or is this an issue and do we need to add some kind of unbind call to remove the closure from the list.

[NachoSoto]

That's absolutely true. We're not using this API ourselves at the moment so I never noticed it.

You unbind idea could work. We'd have to make bind return some sort of opaque token that unbind can use.
An alternative would be making bind take a "context" object and matching the lifetime of that (this could be easily implemented with ReactiveCocoa, I'd need to think how to do it in a simple way without it), and then making that object unowned in the closure to avoid extending its lifetime.

[DanielAsher]

Please consider using RxSwift instead of ReactiveCocoa

[NachoSoto]

Please consider using RxSwift instead of ReactiveCocoa

Uhm why, and what does that have to do with this repo? :)

[bryanjclark]

Thanks for finding this @mac-cain13!

[DanielAsher]

Uhm why, and what does that have to do with this repo? :)

I was considering creating an RxSwiftCommunity project that allowed RxSwift compatibility with SwiftTweaks. There are many good reasons to favour RxSwift over ReactiveCocoa. Your comment gave me the impression you were interested in providing an industry-standard reactive framework in which to base your binding implementation.

[NachoSoto]

There are many good reasons to favour RxSwift over ReactiveCocoa.

I'm sorry but that's A: unfounded, and B: completely irrelevant to this issue and to the library.

Your comment gave me the impression you were interested in providing an industry-standard reactive framework in which to base your binding implementation.

My comment was giving an example of how one could implement this (I mentioned ReactiveCocoa because I happen to know it very well). This framework has nothing to do with reactive programming, so I don't understand why you'd bring this up here. Like I said in my comment, "I'd need to think how to do it in a simple way without it", as I was not implying that SwiftTweaks would add RAC as a dependency.

[bryanjclark]

(I know this is an old thread, but it's still an open issue, so I figured I'd add a comment here.)

I've intentionally avoided adding ReactiveCocoa or RxSwift to the project, so that SwiftTweaks stays simple for anybody to understand!

We use ReactiveCocoa in the Khan Academy iOS app, so it's familiar to us, but that doesn't mean everybody should have to understand it / use it to understand and contribute to SwiftTweaks.

@mac-cain13, thank you for pointing out this bug! I don't have an immediate solution in mind, but definitely open for discussion again.

Thanks!

[mac-cain13]

At the top of my mind I can think of two options (both already discussed above):

1. Make bind return an opaque token that you can pass to the unbind method. (Just like NSNotificationCenter does with blocks)
2. Remove bind from SwiftTweaks and maybe introduce a Rx version in a optional extension lib

I think option 1 would be better since it isn't a breaking change and fixes the issue without losing functionality.

[bryanjclark]

@mac-cain13 I agree - option 1 sounds great for this.

[DanielAsher]

This is a real improvement - thanks!
I've added a small code sample integrating Tweak with RxSwift in the comments of PR #73

[bryanjclark]

Alright, this thing should be closed now - we've got binding tokens in there 😄