The Perils of Task.detached

[nashysolutions]

In the following code, you can see where the detached task causes problems for me.

import Foundation
import Dependencies
import Combine

/// `Arbitrator` is a class that acts as a mediator for value changes. It leverages a `PassthroughSubject` to monitor
/// value changes and invokes a ruling closure when a distinct change is detected.
/// The class uses a debounce mechanism to ensure that rapid fluctuations in the value do not trigger unnecessary actions.
final class Arbitrator<Value: Equatable> {
    
    /// The subject that observes the changes in value.
    private var subject: PassthroughSubject<Value, Never>
    
    /// The subscription to the subject.
    private var subscription: AnyCancellable?
    
    /// A closure that defines the action to be taken when a distinct change in value is detected.
    /// This ruling is executed after the debounce interval, only when the new value differs from the previous value.
    var ruling: (Value) -> Void = { _ in }
    
    /// Initializes the `Arbitrator` with a specific debounce interval.
    /// - Parameter debounceInterval: The time interval (in seconds) to wait before processing a new value.
    init(debounceInterval: TimeInterval) {
        subject = PassthroughSubject()
        
        @Dependency(\.mainRunLoop) var mainRunLoop
        
        subscription = subject
            .debounce(for: .seconds(debounceInterval), scheduler: mainRunLoop)
            .sink { [weak self] newValue in
                guard let self = self else {
                    return
                }
                self.ruling(newValue)
            }
    }
    
    /// Sends a new value to the subject, potentially triggering the ruling closure after the debounce interval.
    /// - Parameter newValue: The new value to be processed.
    func disclose(_ newValue: Value) {
        subject.send(newValue)
    }
}

import Foundation
import Dependencies

final class Manager {
    
    let arbitrator = Arbitrator<Bool>(debounceInterval: 0.2)
    
    var outcome: Bool?
    
    func onAppear() {
        arbitrator.ruling = { [weak self] _ in
            self?.upload()
        }
    }
    
    private func upload() {
        Task.detached {

            @Dependency(\.continuousClock) var continuousClock
            
            try? await continuousClock.sleep(for: .seconds(2))
            self.outcome = true
        }
    }
    
    func disclose(value: Bool) {
        arbitrator.disclose(value)
    }
}

So the following test results in unimplemented for continuousClock.

import XCTest
import Dependencies

@testable import Hello

final class HelloTests: XCTestCase {
    
    func testExample() throws {
        
        let mainRunLoop = RunLoop.test
        withDependencies {
            $0.mainRunLoop = mainRunLoop.eraseToAnyScheduler()
            $0.continuousClock = ImmediateClock()
        } operation: {
            let manager = Manager()
            
            manager.onAppear()
            
            manager.disclose(value: true)
            
            mainRunLoop.advance(by: .seconds(0.2))
            
            let exp = expectation(description: "asdf")
            
            DispatchQueue.main.asyncAfter(deadline: .now() + 1, execute: {
                exp.fulfill()
            })
            
            wait(for: [exp])
            
            XCTAssertEqual(manager.outcome, true)
        }
    }
}

The solution is to move the continuousClock line so it's not within the detached.

@Dependency(\.continuousClock) var continuousClock
    
    private func upload() {
        Task.detached {
            try? await self.continuousClock.sleep(for: .seconds(2))
            self.outcome = true
        }
    }

However, the fix is not so simple in my App code (not the above example code). I can't just move a single line like this. How could I solve the above problem without moving continuousClock ?

[mbrandonw]

However, the fix is not so simple in my App code (not the above example code). I can't just move a single line like this. How could I solve the above problem without moving continuousClock ?

Ideally you should not be using Task.detached at all. They do not inherit task locals, and that is why all dependencies are being lost. Even the docs highly recommend against using Task.detached:

Don’t use a detached task if it’s possible to model the operation using structured concurrency features like child tasks. Child tasks inherit the parent task’s priority and task-local storage, and canceling a parent task automatically cancels all of its child tasks. You need to handle these considerations manually with a detached task.

Is there a reason you can't just use Task { … }?

I also think this code should just generally be refactored to either fully lean into Combine or lean into async/await. Right now it's a mishmash of both, and I think that's causing more indirection than necessary.

[nashysolutions]

Hi Brandon,

Thanks for getting back to me.

I was reluctant to use Thread.sleep in a co-operative thread pool to achieve debouncing. There are less threads available, so I figured it would contribute to resource strangulation. Your thoughts on this would be very much appreciated.

I don't know if I should use Task instead of Task.detached here 😭. Safe handling of the main thread is one aspect of swift concurrency that isn't clear very clear.

Swift's concurrency model does a lot of the heavy lifting when it comes to managing which thread code runs on. By default, tasks that you create using Task { ... } are not bound to any particular thread. They are scheduled on a thread pool managed by Swift, which typically means that they will run on a background thread unless specifically required otherwise (such as updates to the UI). While Swift's Task will typically handle the threading automatically, if you're particularly concerned about even accidental execution on the main thread, consider using environment-specific APIs or patterns that guarantee execution on background threads. However, with the introduction of Swift concurrency, using Task for managing asynchronous operations is generally more idiomatic and recommended.

And the talk Mat gave here makes it even more worrying.

https://www.youtube.com/watch?v=HqjqwW12wpw

Will Task be fine to use instead of Task.detached? I don't have to worry about the main thread being blocked by continuousClock? Thanks.

[mbrandonw]

I was reluctant to use Thread.sleep in a co-operative thread pool to achieve debouncing. There are less threads available, so I figured it would contribute to resource strangulation. Your thoughts on this would be very much appreciated.

There is no mention of Thread.sleep in the code above, so I'm curious why you are mentioning this? You are using clock.sleep, which is perfectly fine to use in the cooperative thread pool.

I don't know if I should use Task instead of Task.detached here 😭. Safe handling of the main thread is one aspect of swift concurrency that isn't clear very clear.

You should use Task { … } seldomly, and you should almost never use Task.detached unless you are specifically not wanting to inherit priority, actor, task locals, etc.

And the talk Mat gave here makes it even more worrying.

What in particular is worrying?

Will Task be fine to use instead of Task.detached? I don't have to worry about the main thread being blocked by continuousClock? Thanks.

Yes, Task will be fine, and no clock.sleep is not blocking. You can see that you are awaiting it, and under the hood it does not do any blocking. However, I will repeat again that you are mixing Combine and async/await together, and in my opinion that makes this code messier than necessary. You should probably just stick to one paradigm.

[verynashy]

Thank you @mbrandonw your feedback is very helpful. I will make the changes you suggested.