CombineCommunity / CombineExt

CombineExt provides a collection of operators, publishers and utilities for Combine, that are not provided by Apple themselves, but are common in other Reactive Frameworks and standards.
https://combine.community
MIT License
1.72k stars 151 forks source link

Memory leak in withLatestFrom operator #121

Closed bharath2020 closed 2 years ago

bharath2020 commented 2 years ago

Hello folks,

We have found a memory leak issue in withLatestFrom operator which is very similar to the memory leak issue found here in ShareReplay.

For better understanding, here is the code snippet

weak var weakSubject1: PassthroughSubject<Int, Never>?
weak var weakSubject2: PassthroughSubject<[Int], Never>?
autoreleasepool {
    let subject1 = PassthroughSubject<Int, Never>()
    let subject2 = PassthroughSubject<[Int], Never>()

    weakSubject1 = subject1
    weakSubject2 = subject2

    let subscritpion = subject1
        .withLatestFrom(subject2)
        .map { (a) -> AnyPublisher<Void, Never> in
            CurrentValueSubject<Void, Never>(()).eraseToAnyPublisher()
        }
        .sink { _ in

        }
    subject2.send([])
    subject1.send(1)
    subject2.send([])
}

weakSubject2 == nil // destroyed as expected.
weakSubject1 == nil // should have been destroyed but not

Expected: subject1 and subject2 must be destroyed after the autorelease pool got drained

Actual: subject1 is held in memory and becomes a zombie even after autorelease pool it belonged to was drained.

Solution: It is very similar to how the share replay memory leak is taken care in this PR

I am happy to put up a PR to fix the issue. Please let me know if there is a better way to handle this.

bharath2020 commented 2 years ago

@freak4pc Thanks for approving and merging the PR. When can I expect the next release? We would want to update our App to include this fix ASAP.

bharath2020 commented 2 years ago

@freak4pc Just bumping this one up. Can you please cut the release or is there a automated process to do so? We would want to release this ASAP as we are using this operator heavily.

freak4pc commented 2 years ago

@bharath2020 You can simply lock your app to the specific commit. I'll try to find time to release soon, but you need to remember contributors don't work full time at cutting releases :) Since it's not an actual blocker there's nothing urgent in cutting the release.

bharath2020 commented 2 years ago

@freak4pc Thanks for the response and totally understand your busy schedule and your contributions 👍 . We avoid locking to specific commits so to pull any patches automatically and noticed there was few commits since last release, hence enquired.

bharath2020 commented 2 years ago

Thanks @freak4pc for releasing this fix. Can I close this issue?

freak4pc commented 2 years ago

Yup, thanks!