1

I do state management locally in my class using a enum. If a particular async work is not done then I kick it off in a Task and then await that task's result. Once the async function completes, it caches the values and updates the state.

Sometimes when the async task is already completed we use the cached value and return early after updating state. It is this path that sometimes throws the system off. The task completes but the next piece of code never picks up the state update. So all subsequent calls give me an error because we are not in an expected state.

So we go from disabled -> starting -> starting -> starting forever, instead of going from disabled -> starting -> started.

Following is a skeleton code I wrote in playground to explain the code structure. I was unable to recreate the prod error in playground though.

import Foundation

struct AsyncResult {
    let resultKey: String
}

enum State {
    case disabled
    case starting(Task<(), Error>)
    case started(AsyncResult)
}

enum ClientErrors: Error {
    case invalidState(State)
}

class Client {
    var state: State = .disabled
    var defaults = UserDefaults.standard
    var key = "resultKey"
    
    private func doSetup() async throws {
        if let result = defaults.string(forKey: key) {
            // Cached value is read, state is updated and we return immediately instead of doing async work.
            print("Reusing saved value")
            state = .started(AsyncResult(resultKey: result))
            return
        }
        
        print("Doing setup")
        
        // Simulate time taken by async work needed to set the key
        try await Task.sleep(nanoseconds: 2_000_000_000)

        try await Task.sleep(nanoseconds: 1_000_000_000)
        
        try await Task.sleep(nanoseconds: 3_000_000_000)

        defaults.set("bar", forKey: key)
        state = .started(AsyncResult(resultKey: "bar"))
    }
    
    func doWork() async throws {
        // Start the async task if not already started
        if case .disabled = state {
            state = .starting(Task { try await doSetup() })
        }
        
        switch state {
        case .disabled:
            throw ClientErrors.invalidState(.disabled)
        case .started(_):
            print("Already setup")
            break
        case .starting(let task):
            print("Waiting for task to complete")
            // Wait on async task
            try await task.value
        }
        
        // Async task should be completed else throw an error
        guard case .started(let result) = state else {
            // This code block runs and throws an error even though a cached value is read and state is updated above
            // From logs I see it reusing saved value but this call and all following calls will fail with the same error
            // after they enter here
            throw ClientErrors.invalidState(state)
        }
        
        print("Will use result: \(result)")
    }
}

UserDefaults.standard.removeObject(forKey: "resultKey")

var client = Client()

// This is meant to simulate what happens in prod code
// I am unable to get it to throw an error and crash in this playground example
for i in 1...100 {
    print("Iteration \(i)")
    try! await client.doWork()
    if Bool.random() {
        print("Resetting client")
        client = Client()
    }
}

Edit: changed from sleep(2) to try await Task.sleep(nanoseconds: 2_000_000_000) to simulate waiting on an async task.

marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Parth
  • 2,682
  • 1
  • 20
  • 39
  • https://stackoverflow.com/questions/73538764/swfit-trouble-running-async-functions-in-background-threads-concurrency/73542522#73542522 – lorem ipsum Sep 03 '22 at 18:23
  • You aren't `await`ing the result. `doSetup` doesn't need to be marked `async` it isn't offering any benefits. – lorem ipsum Sep 03 '22 at 18:25
  • `doSetup` is a proxy of the code in prod that does network calls and calls other async functions. – Parth Sep 03 '22 at 20:15
  • if you aren't using `await` or a `Continuation` there is no purpose for the `async` flag. The function will never leave/return the actor properly. – lorem ipsum Sep 03 '22 at 20:18
  • `sleep(2)` in an async block is invalid. It prevents further progress, which is forbidden. The tool you have to use here is `Task.sleep(2)`. (Do you really mean to sleep for 2 nanoseconds?) Assuming your real code is awaiting (rather than sleeping), then you're probably having reentrancy problems. Remember that if there's an `await`, other code may run before you continue and change state. Also, testing this in a playground is unlikely to work as expected. You need to test in small apps (commandline apps are good). Playgrounds have a very specialized way of evaluating code. – Rob Napier Sep 04 '22 at 00:29
  • (Using sleep breaks your test in that case because sleep blocks the whole thread and prevents other Tasks from executing and likely inter-leaving and breaking your state.) – Rob Napier Sep 04 '22 at 00:30
  • Right, my real code waits on another asyc task and makes network calls. `sleep(2)` was just a proxy for those calls. My `doWork` is the only function that calls `doSetup`. So no other code should be able to modify the state. – Parth Sep 05 '22 at 01:32
  • @RobNapier changed to `try await Task.sleep(nanoseconds: 2_000_000_000)` but still no luck reproing the issue. – Parth Sep 06 '22 at 17:09
  • "then you're probably having reentrancy problems. Remember that if there's an await, other code may run before you continue and change state." The code you're removing is almost certainly where the problem is. "sleep" (even the correct one) doesn't rely on or mutate state, and somewhere you're likely relying on state that gets mutated while you're awaiting. – Rob Napier Sep 06 '22 at 17:48

0 Answers0