DisposeBag leaks externally disposed Disposables

Open fbarbat opened this issue 4 years ago • 1 comments

Short description of the issue:

DisposeBag leaks externally disposed Disposables.

Expected outcome:

DisposeBag should not have references in _disposables to already disposed Disposables. Once any disposable reference from DisposeBag is disposed, it should be removed from the DisposeBag disposables automatically. Running the Debugging Memory Leaks procedure from https://github.com/ReactiveX/RxSwift/blob/main/Documentation/GettingStarted.md should not show an incrementing number of resources if a Disposable added to a DisposeBag was already disposed (for example, if it already was completed). Releasing automatically already disposed Disposables is useful when subscribing to many different disposables from the same class. In other words, I'd like RxSwift as a replacement of a multi delegate pattern in a dynamic context, in which the observer is the delegate of multiple observed objects with shorter lifecycle.

What actually happens:

All the disposables added to the DisposeBag that are disposed later are not removed automatically from the internal list of disposables of the DisposeBag. Running the Debugging Memory Leaks procedure from https://github.com/ReactiveX/RxSwift/blob/main/Documentation/GettingStarted.md shows an incrementing number of resources even if a Disposable added to a DisposeBag was already disposed (for example, if it already was completed).

Self contained code example that reproduces the issue:

    private class Observer {
        private let disposeBag = DisposeBag()

        func watch(observed: Observed) {
            observed.events
                .subscribe(onNext: {
                    print("Something happened!")
                }, onDisposed: {
                    print("Subscription disposed")
                })
                .disposed(by: disposeBag)
        }
    }

    private class Observed {
        // Instead of using a (multi) delegate pattern, I'd like to expose events this way
        var events: Observable<Void> {
            return eventsSubject.asObservable()
        }

        private let eventsSubject = PublishSubject<Void>()

        func doSomething() {
            // Something is done and then an event is emitted

            eventsSubject.onNext(())
        }

        deinit {
            eventsSubject.onCompleted()
            eventsSubject.dispose()
            print("Deinited!")
        }
    }

    func testDisposableLeakOnDisposeBag() {
        let observer = Observer()

        let initialResources: Int32 = Resources.total
        
        print("Resources before \(Resources.total)")
        watchOneObserved(with: observer)
        print("Resources after \(Resources.total)")

        // The observer.disposeBag still retains the observed.publishSubject after the observed object was deinitialized
        // DisposeBag is an always increasing array of disposables even if those disposables are already completed and disposed
        XCTAssertEqual(initialResources, Resources.total)
    }
    
    private func watchOneObserved(with observer: Observer) {
        let observed = Observed()
        observer.watch(observed: observed)
        
        // The observed would emit events for some time and then it is released once this function finishes
    }

RxSwift/RxCocoa/RxBlocking/RxTest version/commit

5.1.1

Platform/Environment

[ ] iOS
[x] macOS
[ ] tvOS
[ ] watchOS
[ ] playgrounds

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

[x] easy, 100% repro
[ ] sometimes, 10%-100%
[ ] hard, 2% - 10%
[ ] extremely hard, %0 - 2%

Xcode version:

  12.5.1

Installation method:

[x] CocoaPods
[ ] Carthage
[ ] Git submodules

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

[ ] yes (which ones)
[x] no

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

[ ] just starting
[ ] I have a small code base
[x] I have a significant code base

Sep 16 '21 01:09 fbarbat

Short description of the issue:

DisposeBag leaks externally disposed Disposables.

Expected outcome:

DisposeBag should not have references in _disposables to already disposed Disposables. Once any disposable reference from DisposeBag is disposed, it should be removed from the DisposeBag disposables automatically. Running the Debugging Memory Leaks procedure from https://github.com/ReactiveX/RxSwift/blob/main/Documentation/GettingStarted.md should not show an incrementing number of resources if a Disposable added to a DisposeBag was already disposed (for example, if it already was completed). Releasing automatically already disposed Disposables is useful when subscribing to many different disposables from the same class. In other words, I'd like RxSwift as a replacement of a multi delegate pattern in a dynamic context, in which the observer is the delegate of multiple observed objects with shorter lifecycle.

What actually happens:

All the disposables added to the DisposeBag that are disposed later are not removed automatically from the internal list of disposables of the DisposeBag. Running the Debugging Memory Leaks procedure from https://github.com/ReactiveX/RxSwift/blob/main/Documentation/GettingStarted.md shows an incrementing number of resources even if a Disposable added to a DisposeBag was already disposed (for example, if it already was completed).

Self contained code example that reproduces the issue:
    private class Observer {
        private let disposeBag = DisposeBag()

        func watch(observed: Observed) {
            observed.events
                .subscribe(onNext: {
                    print("Something happened!")
                }, onDisposed: {
                    print("Subscription disposed")
                })
                .disposed(by: disposeBag)
        }
    }

    private class Observed {
        // Instead of using a (multi) delegate pattern, I'd like to expose events this way
        var events: Observable<Void> {
            return eventsSubject.asObservable()
        }

        private let eventsSubject = PublishSubject<Void>()

        func doSomething() {
            // Something is done and then an event is emitted

            eventsSubject.onNext(())
        }

        deinit {
            eventsSubject.onCompleted()
            eventsSubject.dispose()
            print("Deinited!")
        }
    }

    func testDisposableLeakOnDisposeBag() {
        let observer = Observer()

        let initialResources: Int32 = Resources.total
        
        print("Resources before \(Resources.total)")
        watchOneObserved(with: observer)
        print("Resources after \(Resources.total)")

        // The observer.disposeBag still retains the observed.publishSubject after the observed object was deinitialized
        // DisposeBag is an always increasing array of disposables even if those disposables are already completed and disposed
        XCTAssertEqual(initialResources, Resources.total)
    }
    
    private func watchOneObserved(with observer: Observer) {
        let observed = Observed()
        observer.watch(observed: observed)
        
        // The observed would emit events for some time and then it is released once this function finishes
    }
RxSwift/RxCocoa/RxBlocking/RxTest version/commit

5.1.1

Platform/Environment

[ ] iOS

[x] macOS

[ ] tvOS

[ ] watchOS

[ ] playgrounds

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

[x] easy, 100% repro

[ ] sometimes, 10%-100%

[ ] hard, 2% - 10%

[ ] extremely hard, %0 - 2%

Xcode version:
  12.5.1
Installation method:

[x] CocoaPods

[ ] Carthage

[ ] Git submodules

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

[ ] yes (which ones)

[x] no

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

[ ] just starting

[ ] I have a small code base

[x] I have a significant code base

Sep 29 '21 17:09 davidofell5