3

With Swift 3.1 on XCode 8.3, running the following code with the Thread Sanitizer finds a data race (see the write and read comments in the code):

  private func incrementAsync() {
    let item = DispatchWorkItem { [weak self] in
      guard let strongSelf = self else { return }
      strongSelf.x += 1 // <--- the write

      // Uncomment following line and there's no race, probably because print introduces a barrier
      //print("> DispatchWorkItem done")
    }
    item.notify(queue: .main) { [weak self] in
      guard let strongSelf = self else { return }
      print("> \(strongSelf.x)") // <--- the read
    }

    DispatchQueue.global(qos: .background).async(execute: item)
  }

This seems pretty strange to me as the documentation for the DispatchWorkItem mentions that it allows:

getting notified about their completion

which implies that the notify callback is called once the work item's execution is done.

So I would expect that there would be a happens-before relationship between the DispatchWorkItem's work closure and its notify closure. What would be the correct way, if any, to use a DispatchWorkItem with a registered notify callback like this that wouldn't trigger the Thread Sanitizer error?

I tried registering the notify with item.notify(flags: .barrier, queue: .main) ... but the race persisted (probably because the flag only applies to the same queue, documentation is sparse on what the .barrier flag does). But even calling notify on the same (background) queue as the work item's execution, with the flags: .barrier, results in a race.

If you wanna try this out, I published the complete XCode project on github here: https://github.com/mna/TestDispatchNotify

There's a TestDispatchNotify scheme that builds the app without tsan, and TestDispatchNotify+Tsan with the Thread Sanitizer activated.

Thanks, Martin

mna
  • 22,989
  • 6
  • 46
  • 49
  • 1
    This can no longer be manifested. They’ve been revising and enhancing the thread sanitizer, so I suspect that this was merely a bug in an early iteration of TSAN. – Rob Jan 06 '19 at 17:17

2 Answers2

1

EDIT (2019-01-07): As mentioned by @Rob in a comment on the question, this can't be reproduced anymore with recent versions of Xcode/Foundation (I don't have Xcode installed anymore, I won't guess a version number). There is no workaround required.


Well looks like I found out. Using a DispatchGroup.notify to get notified when the group's dispatched items have completed, instead of DispatchWorkItem.notify, avoids the data race. Here's the same-ish snippet without the data race:

  private func incrementAsync() {
    let queue = DispatchQueue.global(qos: .background)

    let item = DispatchWorkItem { [weak self] in
      guard let strongSelf = self else { return }
      strongSelf.x += 1
    }

    let group = DispatchGroup()
    group.notify(queue: .main) { [weak self] in
      guard let strongSelf = self else { return }
      print("> \(strongSelf.x)")
    }
    queue.async(group: group, execute: item)
  }

So DispatchGroup introduces a happens-before relationship and notify is safely called after the threads (in this case, a single async work item) finished execution, while DispatchWorkItem.notify doesn't offer this guarantee.

mna
  • 22,989
  • 6
  • 46
  • 49
  • `DispatchWorkItem` should guarantee no race for a _single_ execution. The race for your situation would seem to come from executing the Block multiple times and then notifying on a separate queue (that's not synchronized with respect to the work queue). If you haven't seen it, I think the explanation of the behavior in the docs for `dispatch_block_notify()` is better. – jscs Mar 31 '17 at 02:12
  • It is a single execution, though. Each time the method is called, it creates a new DispatchWorkItem that executes just once. Thanks for the heads-up about the docs, it does mention using DispatchGroup there! Looks like the swift docs haven't quite caught up in some places. – mna Mar 31 '17 at 02:20
  • Hm, yes, you're absolutely right. I got myself mixed up puzzling it over. – jscs Mar 31 '17 at 03:06
  • @mna please see my "answer". It is not about thread-safe data read, write. I just would like to explain why you are wrong and why you answer should not be accepted as a solution. – user3441734 Jan 06 '19 at 14:44
0
import Foundation
import PlaygroundSupport
PlaygroundPage.current.needsIndefiniteExecution = true

var job = DispatchWorkItem {
    for i in 0..<3 {
        DispatchQueue.main.async {
            print("job", i)
        }
    }
    DispatchQueue.main.async {
        print("job done")
    }
}
job.notify(queue: .main) {
    print("job notify")
}

DispatchQueue.global(qos: .background).asyncAfter(deadline: .now(), execute: job)
usleep(100)
job.cancel()

if you guess that this snippet prints out

job 0
job 1
job 2
job done
job notify

you are absolutely right! increase a deadLine ...

DispatchQueue.global(qos: .background).asyncAfter(deadline: .now() + 0.01, execute: job)

and you've got

job notify

even though the job executes never

notify has nothing with synchronization of any data captured by DispatchWorkItem's closure.

Let try this example with DispatchGroup!

import Foundation
import PlaygroundSupport
PlaygroundPage.current.needsIndefiniteExecution = true


let group = DispatchGroup()
group.notify(queue: .main) {
    print("group notify")
}

And see the result

group notify

!!! WTF !!! Do you still think you solved the race in your code? To synchronize any read, write ... use the serial queue, barrier, or semaphore. Dispatch group is totally different beast :-) With dispatch groups you can group together multiple tasks and either wait for them to complete or receive a notification once they complete.

import Foundation
import PlaygroundSupport
PlaygroundPage.current.needsIndefiniteExecution = true

let job1 = DispatchWorkItem {
    sleep(1)
    DispatchQueue.main.async {
        print("job 1 done")
    }
}
let job2 = DispatchWorkItem {
    sleep(2)
    DispatchQueue.main.async {
        print("job 2 done")
    }
}
let group = DispatchGroup()
DispatchQueue.global(qos: .background).async(group: group, execute: job1)
DispatchQueue.global(qos: .background).async(group: group, execute: job2)

print("line1")
group.notify(queue: .main) {
    print("group notify")
}
print("line2")

prints

line1
line2
job 1 done
job 2 done
group notify
user3441734
  • 16,722
  • 2
  • 40
  • 59