RxProperty icon indicating copy to clipboard operation
RxProperty copied to clipboard

Possible undisposed subscription memory leak

Open elfenlaid opened this issue 4 years ago • 2 comments

I wonder whether ignoring the bind`s disposable leads to a leak.

    /// Initializes with `initial` element and then `observable`.
    public init(initial: E, then observable: Observable<E>) {
        _behaviorRelay = BehaviorRelay(value: initial)

        _ = observable
            .bind(to: _behaviorRelay)
            // .disposed(by: disposeBag)    // Comment-Out: Don't dispose when `property` is deallocated
    }

Specifically if Observable is never-ending one?

elfenlaid avatar Jun 29 '20 06:06 elfenlaid

I think it's because after Property itself is deallocated existing subscriptions of the property will stop working (https://github.com/inamiy/RxProperty/pull/6).

I also experienced memory leaks in RxProperty and I ended up with writing another Property implementation that just wraps Driver:

import Foundation
import RxSwift
import RxCocoa

class Property<Element> {
    private var _value: Element!
    private let _driver: Driver<Element>
    private let disposeBag = DisposeBag()

    var value: Element {
        return _value
    }

    init(_ driver: Driver<Element>) {
        _driver = driver
        driver.drive(onNext: { [unowned self] in
            self._value = $0
        }).disposed(by: disposeBag)
    }

    func asDriver() -> Driver<Element> {
        return _driver
    }
}

seanchas116 avatar Jul 14 '20 09:07 seanchas116

Ah, some dangerous shenanigans around explicitly unwrapped value 🙈

We accepted property nature as is and ended up with the next:

/// `Property` is a read-only wrapper for `BehaviorRelay`.
///
/// Unlike `BehaviorRelay` it can't accept values
public final class Property<Element> {
    public var value: Element {
        relay.value
    }

    public convenience init(value: Element) {
        self.init(relay: BehaviorRelay(value: value))
    }

    public init(relay: BehaviorRelay<Element>) {
        self.relay = relay
    }
    
    public func bind(to handler: @escaping (Element) -> Void) -> Disposable {
        relay.observeOn(MainScheduler.instance).subscribe(onNext: handler)
    }

    public func asObservable() -> Observable<Element> {
        relay.observeOn(MainScheduler.instance)
    }

    private let relay: BehaviorRelay<Element>
}

public extension BehaviorRelay {
    func asProperty() -> Property<Element> {
        Property(relay: self)
    }
}

So viewModel uses BehaviorRelay under the hood and exposes properties as a public interface.

The saddest thing here is the loss of composability. Property can't be mapped, filtered, etc. Which is the bread and butter of the Rx approach.

Though Relay and Subject are deviations from core primitives on their own and it's only an unavoidable consequence. Yet, ReactiveCocoa has solved the same issue with its Property/MutableProperty. The hope is not lost.

elfenlaid avatar Jul 14 '20 10:07 elfenlaid