1

Hi everyone,

I am playing around with Swift's concurrency and as an exercise, I wanted to make some kind of file downloader that could concurrently download multiple files.
I came up with a (surprisingly) working solution but I am unsure if this is a good solution or if there is any issue with what I've done.

Each download task is represented by the following (simplified) struct:

struct FileTask {
    let url: URL
    // ...
}

I then have the following functions that, given an array of tasks and an allowed number of concurrent downloads, will chunk the array and download the files using an async task group:

func download(tasks: [FileTask], maxConcurrent: Int = 0) async throws {
    guard !tasks.isEmpty else {
        return
    }

    let chunks: [[FileTask]]
    if maxConcurrent == 0 {
        chunks = [tasks]
    } else {
        chunks = stride(from: 0, to: tasks.count, by: maxConcurrent).map { index in
            let upperLimit = min(index + maxConcurrent, tasks.count)
            return Array(
                tasks[index..<upperLimit]
            )
        }
    }   

    for chunk in chunks {
        await withThrowingTaskGroup(of: Void.self) { group in
            for task in chunk {
                group.addTask {
                    // function download(task: FileTask) -> async throws
                    // This function handles the network call and writes the file to disk
                    try await download(task: task)
                }
            }
        }
    }
}

I am very unsure about the for chunk in chunks with an await inside, is this an anti-pattern? Is there any limitation or caveat I should know about using this solution?

2

Two observations:

  1. Your approach with for chunk in chunks doesn't keep maxConcurrent tasks in flight at all times. Rather, it will wait for all tasks in the first chunk to finish before starting with the second chunk, and so on. Is this what you want?

    Here's an alternative approach from @Jon_Shier to set an upper bound on the number of concurrent child tasks in a task group: Trying to implement basic concurrent code with actors - #12 by Jon_Shier

  2. Your code silently ignores any errors thrown by try await download(task: task). Because you don't explicitly await the results produced by the child tasks, the task group will implicitly await its child tasks when it goes out of scope. Any errors thrown by the child tasks will be thrown away.

    The compiler gives you a tiny hint that this is happening by allowing you to write await withThrowingTaskGroup and not forcing you to write try await here.

3

Very insightful response, thanks.

  1. This was indeed the expected behaviour at first, I wasn't aware you could do for await _ in group, this makes a lot more sense

  2. I was indeed wondering why try await withThrowingTaskGroup raised a warning, very interesting. I however fail to understand how / where an error thrown by a child task should be handled. My best guess is wrapping the try await download(task: task) in a do { } catch { }

4

You can iterate over the group's results after the loop where you enqueue the tasks. Something like this:

// Now `try` is required here
try await withThrowingTaskGroup(of: Void.self) { group in
    for task in chunk {
        group.addTask {
            try await download(task: task)
        }
    }
    // Wrap this in do { } catch { } if you don't want
    // child task errors to propagate:
    for try await _ in group {
        // Nothing to do here because the child tasks don't return anything.
    }
}

Written like this, any error thrown by a child task will propagate to the outside, and the task group will automatically cancel other child tasks that are still in flight.

If that is not what you want, you can wrap the second loop in a do { } catch { } and handle errors locally. From the caller's perspective, this approach is more or less what you have now, but the code makes it explicit that child task errors aren't propagated.

Edit: code fixes

3 Likes
5

This makes a lot of sense, I prefer to not cancel the entire group but having the possibility to do so is great. Having played around with both solutions, I enjoy the possibilities they offer.

Thank you a lot for the explanations, they were very educational and insightful !

1 Like