CODEWITHSUNDEEP
iosswiftmemory-managementmemory-leaksrx-swift
Bawenang Rukmoko Pardian Putra
Bawenang Rukmoko Pardian Putra

Reputation: 1408

Memory leak after refactoring `compactMap` for RxSwift.Observable from closure to function

I found a probable leak in my code because of a unit test which tests whether a dependency is called at deinit:

func testDeinit_ShouldStopMinutesTicker() throws {
    let minutesTicker = MockMinutesTicker(elapsedTicksAfterStart: [(), (), ()])
    var viewModel: AppointmentViewModel? = createAppointmentViewModel(minutesTicker: minutesTicker)
    viewModel = nil
    
    XCTAssertTrue(minutesTicker.isStopCalled)
}

This test is usually green. But when I refactored this:

func selectAppointment(index: Int) {
    selectedCellParamRelay.accept(index)
}

private func setupCellParamSelection() {
    selectedCellParamRelay
        .withLatestFrom(sortedCellParams) { ($0, $1) }
        .compactMap { [weak self] index, cellParams in
            guard let `self` = self,
                  let selectedParam = cellParams[safe: index],
                  self.isUpcomingAppointment(selectedParam) else { return nil }
            
            return selectedParam
            
        }
        .bind(to: upcomingCellParamRelay)
        .disposed(by: disposeBag)
}

Into this:

func selectAppointment(index: Int) {
    selectedCellParamRelay.accept(index)
}

private func setupCellParamSelection() {
    selectedCellParamRelay
        .withLatestFrom(sortedCellParams) { ($0, $1) }
        .compactMap(selectAppointment(index:from:))
        .bind(to: upcomingCellParamRelay)
        .disposed(by: disposeBag)
}

private func selectAppointment(
    index: Int,
    from cellParams: [AppointmentCellParam]
) throws -> AppointmentCellParam? {
    guard let selectedParam = cellParams[safe: index],
          isUpcomingAppointment(selectedParam) else { return nil }

    return selectedParam
}

private func isUpcomingAppointment(_ appointment: AppointmentCellParam) -> Bool {
    return appointment.status != .missed && appointment.status != .finished
}

It becomes red and the deinit is not called at all.

The setupCellParamSelection func is being called from the init to setup the event handler for selecting any appointment cell by index. The sortedCellParams is a relay that is emitted by a dependency that in turn will get the values from the backend.

Can you help me figure out what went wrong with the second code?

Thanks.

Upvotes: 1

Views: 401

Answers (2)

Anton Tropashko
Anton Tropashko

Reputation: 5806

On top of the accepted answer: CompactMap itself seems to leak:

override func subscribe(observer: AnonymousObserver<Result>) -> Disposable {
    let disposable1 = super.subscribe(observer: observer)
    let disposable2 = source.subscribe(
        onNext: {
            guard let mappedElement = self.transform($0) else { return }
            self.dispatch(event: .next(mappedElement))
        },

Upvotes: 0

Daniel T.
Daniel T.

Reputation: 33967

Yes, because .compactMap(selectAppointment(index:from:)) needs to hold self in order to call selectAppointment even though the function doesn't need self.

The solution here is to move isUpcomingAppointment(_:) and selectAppointment(index:from:) outside of the class or as static functions within the class.

Upvotes: 2

Related Questions