Search code examples
iosswiftmemory-managementmemory-leaksrx-swift

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.


Solution

  • 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.