JensRavens/Interstellar

Memory leak when map'ing signals

vytis opened this issue · 4 comments

vytis commented

It seems that after doing .map to a signal the newly created signal never gets deallocated. For example:

import Interstellar

let text = Signal<String>()

func doNothing(signal: Signal<String>) {
    let leakedSignal = signal.map { _ in
        return "I am silent"
    }.next { text in
        print(text)
    }
}

doNothing(text)

text.update("Hey")
text.update("Hey")

In this case I would expect nothing to be printed to the console because I update text signal and there are no subscribers in the current scope. However in reality I get "I am silent" two times because leakedSignal is not released after doNothing is finished.

For this particular map capturing signal weakly solves the issue:

public func map<U>(f: T -> U) -> Signal<U> {
    let signal = Signal<U>()
    subscribe { [weak signal] result in
        signal?.update(result.map(f))
    }
    return signal
}

I guess this should be the case for all variants of map and possibly other operators.

Yes, all signals keep a strong reference to child-signals. But this is somewhat on purpose, otherwise you would have to keep a reference to all signal-chains. Just imagine this case:

textField.textSignal.map(trim).map(downcase).flatMap(sendNetworkRequest)

This chain would never execute because there is no way to capture the signal after the first map.

vytis commented

That is true, didn't realise that chaining wouldn't work. In your example I would expect that you should only need to keep the reference to the last signal created. Not sure how it would be possible to implement this though.

So that also means that any values sent through the signal would also leak? Didn't do any tests but it could have a big impact on overall memory consumption

It's not actually leaking - the memory is still accessible. As soon as you let go of the initiating signal, the whole graph of signals collapses and get's deallocated.

RxSwift is working around that by having dispose bags, but i would like to keep this implementation is small as possible, so this concept might be overkill.

Also retaining without having a reference is quite a nice feature if you use it with the new subscribeOnce-option that is coming to observable in 2.0 (so signal can be used as a replacement for futures/promises).

vytis commented

Ah ok, then it's not a big problem if everything gets released in the end. Thanks for a nice explanation!