1
func doSomething() -> Int {
    var sum = 0
    let increaseWork = DispatchWorkItem {
        sum = sum + 100 //point 1
    }
    DispatchQueue.global().async(execute:increaseWork)
    increaseWork.wait()
    return sum //point 2
}

Thread Sanitizer is saying that there is race condition between point 1 and point 2. But I don't think there is any race condition as increaseWork.wait() is blocking call and it will not pass until the closure is executed.

Tarun Chawla
  • 508
  • 4
  • 17

1 Answers1

1

There could be a race condition between those 2 points. Imagine what happens if you execute the doSomething() function on 2 separate threads like this:

  1. The first thread executes the increaseWork() closure and finishes it. It's at the line with wait right now
  2. The second thread starts executing and hits the async execute instruction
  3. The first thread hits the return instruction as the wait can continue
  4. At the same time, the closure from the second is scheduled and executed

At this point, you can't tell for sure what is executed first: the sum = sum + 100 from the second thread or the return sum from the first one.

The idea is that sum is a shared resource which is not synchronised, so, in theory, that could be a race condition. Even if you took care that such thing does not happen, the Thread Sanitizer detects a possible race condition as it does not know whether you start a single thread or you execute doSomething() function from 100 different threads at the same time.

UPDATE:

As I missed the fact that the sum variable is local, the above explanation does not answer the current question. The scenario described would never take place in the given piece of code.

But, even though the sum is a local variable, due to the fact it is used and retained inside a closure, it will be allocated on heap, rather than on stack. That's because the Swift compiler cannot determine whether the closure will be finished its execution before the doSomething() function return or not.

Why? Because the closure is passed to a constructor which behaves like an @escaping parameter. It implies that there is no guarantee when the closure will be executed, thus all the variables retained by the closure must be allocated on heap for safety. Not knowing when the closure will be executed, the Thread Sanitizer cannot determine that the return sum statement will be indeed executed after the closure finishes.

So even though here we can be sure that no race condition can happen, the Thread Sanitizer raises an alarm, as it cannot determine it cannot happen.

Vlad Rusu
  • 1,414
  • 12
  • 17
  • sum is a local variable, even if you call this function from 100 threads, each thread has their own copy of sum and it is not a shared resource, right? It is shared inside the doSomething() only in the same thread. – Tarun Chawla Apr 15 '20 at 08:03
  • Ok, missed it was local. The idea is that if the `sum` is used in a closure, it will be allocated on heap. We know that it's not necessary, as the function will never return before the closure ends, but that's something the Swift compiler does not know. So the Sanitizer assumes everything is possible. – Vlad Rusu Apr 15 '20 at 08:09
  • so this means I can safely assume this is a false alarm? – Tarun Chawla Apr 15 '20 at 08:11
  • 1
    Yes, in this case it's safe – Vlad Rusu Apr 15 '20 at 08:15
  • Am also thinking on same line, will accept this as answer after checking more on this. Thanks! – Tarun Chawla Apr 15 '20 at 08:17
  • Sorry I can't accept unless you modify it as in current form it is not a correct answer. – Tarun Chawla Apr 15 '20 at 08:20
  • Hey one more thing, is there any specific reason why compiler ignored the wait call? – Tarun Chawla Apr 15 '20 at 09:25
  • I don't know for sure about a specific reason, but my simple guess would be that it's quite hard to conclude that this code is indeed thread safe. You can call `increaseWork.perform()` anywhere else and all these edge cases are hard to be caught by the sanitizer. So if it cannot be 100% sure that all of the code is safe, it shows a potential issue. So better said, it did not ignore the wait call, but rather it was not enough to tell if the race condition might happen or not. – Vlad Rusu Apr 15 '20 at 09:38