Does WWDC21 "Swift concurrency: Update a sample app" video has data race bug?

Hi. I've been watching this video that probably many of you watched and noticed something strange during updates to the save functionality.

There is this snippet of code used in the result functionality (I cleaned it up and left only essentials for this question)

public func addDrink(mgCaffeine: Double, onDate date: Date) {
	…
    currentDrinks = drinks
        
    Task {
        await self.drinksUpdated()
    }
}

private func drinksUpdated() async {
	...
    await store.save(currentDrinks)
}

addDrink is always called on the main actor so we are safe with updating instance variable currentDrinks. Everything is fine here. But then afterwards we schedule a task to perform saving of the current state. We await for the store because it is an actor and this means suspension point. Now imagine that there are several calls happening to addDrink faster then store is able to save. This will mean that there will be several Tasks waiting for the access to the store. And there is no guarantee which task will run first so this means that we can't be sure which state of the currentDrinks will get written to the store last.

The same example in a bit different words.

  1. We have 0 values in currentDrinks
  2. addDrink is getting called
  3. Now we have 1 value in currentDrinks
  4. We fire a Task (lets call it Task1) to save to the store and it starts running
  5. addDrink is getting called again
  6. Now we have 2 values in currentDrinks
  7. We schedule another Task (Task2) to save to the store while currentDrinks has 2 values and it suspends at the await store.save(currentDrinks) (first task is still running)
  8. addDrink is getting called again
  9. Now we have 3 values in currentDrinks
  10. We schedule another Task (Task3) to save to the store while currentDrinks has 3 values and it suspends at the await store.save(currentDrinks) (first task is still running)
  11. Task1 finishes
  12. Now one of the tasks from Task2 and Task3 will get the chance to run but we don't control which one.
  13. Task3 runs first and saves 3 values into store
  14. Task2 runs and saves 2 values into store overwriting what Task3 did
  15. We end up with 3 values in currentDrinks and 2 values in store

If we will relaunch app after this, we will basically loose 3rd drink that we added on the previous run.

Am I correct that this sample code contains a bug or am I missing something about Swift Concurrency?

Post not yet marked as solved Up vote post of belyakov_e Down vote post of belyakov_e
145 views
Posted by
belyakov_e
Reply
Add a Comment

Replies

I don't see a race condition here. drinksUpdated doesn't capture the value of currentDrinks, it retrieves the value from self whenever the function executes. Since the Task closure is isolated to the main thread in this case (because addDrink is isolated to the main thread, and this kind of task inherits the execution context where it is created), retrieving the value of currentDrinks is safe.

So, in step 14 of your scenario, 3 values are written back to the store, not 2, and they're the same 3 that were written in step 13. There's a little bit of inefficiency here, but no race condition.

If, hypothetically, the value of currentDrinks was passed as a parameter into drinksUpdate, then that would be a race condition, but this code doesn't make that mistake.

Posted by
Polyphonic
Up vote reply of Polyphonic Down vote reply of Polyphonic

  • Unfortunately my comment didn't fit into comment section so I posted it as the reply. Please see it below.

    —
    belyakov_e
Add a Comment

Unfortunately it retrieves value from self not when function executes but when suspension happens and at that point there are still 2 values in the array. I've comprised a small sample project to illustrate this behaviour. When Task2 runs it uses value that it has retrieved from self when it suspended (and at that point there were 2 values).

Here is the sample code.

actor A {
    func perform(val: Int) {
        sleep(2)
        print(val)
    }
}

@MainActor
class ViewController: UIViewController {
    let a = A()
    var v = 0

    let priorities: [TaskPriority] = [.low, .medium, .high]

    @IBAction func doStuff(_ sender: Any) {
        v += 1
        Task(priority: priorities[v - 1]) {
            await self.b()
        }
    }

    func b() async{
        print("before: \(v)")
        await self.a.perform(val: v)
    }
}

doStuff is the button handler. We tap button 3 times in a row and the output is

before: 1
before: 2
before: 3
1
3
2

perform method of actor A simulates our long running save operation. Priorities of Tasks are used here solely to demonstrate that order of execution can be different.

Here is the screenshot of this code running

Posted by
belyakov_e
Up vote reply of belyakov_e Down vote reply of belyakov_e
Add a Comment