Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when using Combine Publisher as AsyncSequence #123

Closed
Bradleycorn opened this issue Jul 27, 2023 · 9 comments · Fixed by #126
Closed

Crash when using Combine Publisher as AsyncSequence #123

Bradleycorn opened this issue Jul 27, 2023 · 9 comments · Fixed by #126

Comments

@Bradleycorn
Copy link

I'm using version: 1.0.0-ALPHA-6

I have a Publisher, and I'm converting to an AsyncSequence like this:

let seq = $selectedRace
    .compactMap{$0} // don't emit nil values
    .removeDuplicates { prev, next in
        prev.raceId.description == next.raceId.description
    }
    .map { createPublisher(for: self.trackRepo.getRunners(id: $0.raceId)) }
    .switchToLatest()
    .values



for try await runners in seq {
        print("Got more runners")
        runnerList = runners
}

In the above, $selectedRace is the publisher for an @Published property in an @ObservableObject.
When the published property's value changes, I need to call my Repo with the new value (which returns a Flow) to get a stream of updated data.

When the above code runs, it works briefly (I get "Got more runners" a few times in the console output), but after a few seconds, it crashes. It seems to fail in NativeFlowSubscription, on line 49 of Publisher.swift (see screenshot below).

FWIW, in other areas of my app, I use an AsyncSequence with the same kotlin call (asyncSequence(for: self.trackRepo.getRunners(id: raceId)) and it works fine.

image
@rickclephas
Copy link
Owner

Hi, thanks for reporting this!
Could you possibly share a reproduction sample?
I am guessing $selectedRace plays a critical role in this issue.

@Bradleycorn
Copy link
Author

@rickclephas It's part of a fairly big project. Let me see if I can put together a small demo to reproduce the issue. It might take me a few days.
In the meantime, here's a larger snippet of the code involved, in case that helps.

class ProgramViewModel: ObservableObject {
 
    @Published var selectedRace: Race? = nil
    
    @Published var runnerList: [Runner] = []

    let races: [Race]

    @MainActor // So that the uiState (which drives the views in the ui) is updated on the main thread.
    func loadProgram() async {
        Task { [self] in
             do {
                 let seq = $selectedRace
                    .compactMap{$0} // don't emit nil values
                    .removeDuplicates { prev, next in
                        prev.raceId.description == next.raceId.description
                    }
                    .map { createPublisher(for: self.trackRepo.getRunners(id: $0.raceId)) }
                    .switchToLatest()
                    .values
 
                for try await runners in seq {
                    print("Got more runners")
                    runnerList = runners
                }
            } catch {
                print("Error!!!!")
            }
        }
    }
}

In the View, selectedRace is used as a binding for a dropdown of Races. When a race is selected in the dropdown, the binding updates the value of selectedRace. Also, a .task modifier on the View calls the ProgramViewModel's loadProgram function.

struct ProgramView: View {
    @StateObject var viewModel = ProgramViewModel()
    
    var body: some View {
         VStack {
              Picker("Select a Race", selection: $viewModel.race) {
                  ForEach(viewModel.races, id: \.self) {
                          Text("Race \($0.raceNumber)")
                 }
             }
             
             List { ... this is a list that displays the runners in viewModel.runnerList }
         }
         .task { await viewModel.loadProgram() }
    }
}

When the view loads up, I pick a race from the dropdown, and everything works for a period of time. Usually a few seconds, sometimes a minute or so. And then I get the crash/error. I don't have to select a different race or anything.

@Bradleycorn
Copy link
Author

@rickclephas it's also worth noting ... the Kotlin Flow (trackRepo.getRunners(id: RaceId)) is a Flow that will "never" finish/terminate until canceled. It's polling an API endpoint every X seconds to get updated data (and emit it to the Flow).

@rickclephas
Copy link
Owner

Hi @Bradleycorn thanks for the additional details. Unfortunately I wasn't able to reproduce the issue.
I am probably missing something in my attempt to reproduce this, so it would be great to have a reproducer for this.

@Bradleycorn
Copy link
Author

@rickclephas Sorry, I got sidetracked on some things. I will try to get a demo project together asap.

@Bradleycorn
Copy link
Author

@rickclephas I have created a demo project. You can find it here:
https://github.com/Bradleycorn/PublisherDemo

To reproduce the issue, Build and run the included iOS App (I've been using a Simulator on iOS 16.4).
When the app loads choose a value from the displayed Picker. You may have to wait a few seconds and/or select a few Picker values to get it to crash. But it should crash eventually. For me it usually crashes right away.

@rickclephas rickclephas changed the title Crash when using Publisher with a Flow Crash when using Combine Publisher as AsyncSequence Aug 9, 2023
rickclephas added a commit that referenced this issue Aug 9, 2023
…sher-demand

Support Combine Publisher back pressure
@rickclephas
Copy link
Owner

@Bradleycorn thanks again for reporting this and providing the demo project.
Turns out the issue was with the unimplemented demand logic.
.values will request a singel value at a time which wasn't being respected.
The demand/back pressure logic has been added, fixing this crash. I'll publish a new release somewhere this week.

@Bradleycorn
Copy link
Author

Thanks @rickclephas !

@rickclephas
Copy link
Owner

FYI the fix has been released in v1.0.0-ALPHA-15.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants