Why does DispatchWorkItem notify crash?

1.4k views Asked by At

I've just started to learn a little bit more about Grand Central Dispatch in the Swift programming language.

I followed a tutorial online to better understand GCD and tried various examples of usage...

in the section about Work Item I wrote the following code :

func useWorkItem() {
    var value = 10
    let workItem = DispatchWorkItem {
        value += 5
    }

    workItem.perform()
    let queue = DispatchQueue.global(qos: .utility)
    queue.async(execute: workItem)

    workItem.notify(queue: DispatchQueue.main) {
        print("value = ", value)
    }
}

the code basically perform the workItem in two different queues (the main and global queue) and when the work item finish running in both queues I get the result.

the output of the code above is : 20.

when I tried to manipulate the code a little bit and added another Queue to the mix and ran the same workItem with the same qos as the global queue (.utility), like so :

 func useWorkItem() {
    var value = 10
    let workItem = DispatchWorkItem {
        value += 5
    }

    workItem.perform()
    let queue = DispatchQueue.global(qos: .utility)
    queue.async(execute: workItem)

    let que = DispatchQueue(label: "com.appcoda.delayqueue1", qos: .utility)
    que.async(execute: workItem)

    workItem.notify(queue: DispatchQueue.main) {
        print("value = ", value)
    }
}

the app crashes.

but when I change the order of the commands so I move the workItem.notify method to the beginning of the method, the app works and give me correct output which is 25 :

func useWorkItem() {
    var value = 10
    let workItem = DispatchWorkItem {
        value += 5
    }

    workItem.notify(queue: DispatchQueue.main) {
        print("value = ", value)
    }

    workItem.perform()
    let queue = DispatchQueue.global(qos: .utility)
    queue.async(execute: workItem)
    let que = DispatchQueue(label: "com.appcoda.delayqueue1", qos: .utility)
    que.async(execute: workItem)
}

can anyone please help understand how the .notify() method really works ? and why the order of the commands made a difference ?

thanks a lot in advance...

1

There are 1 answers

1
Rob On BEST ANSWER

That first example you share (which I gather is directly from the tutorial) is not well written for a couple of reasons:

  1. It's updating a variable from multiple threads. That is an inherently non thread-safe process. It turns out that for reasons not worth outlining here, it's not technically an issue in the author's original example, but it is a very fragile design, illustrated by the non-thread-safe behavior quickly manifested in your subsequent examples.

    One should always synchronize access to a variable if manipulating it from multiple threads. You can use a dedicated serial queue for this, a NSLock, reader-writer pattern, or other patterns. While I'd often use another GCD queue for the synchronization, I think that's going to be confusing when we're focusing on the GCD behavior of DispatchWorkItem on various queues, so in my example below, I'll use NSLock to synchronize access, calling lock() before I try to use value and unlock when I'm done.

  2. You say that first example displays "20". That's a mere accident of timing. If you change it to say ...

    let workItem = DispatchWorkItem {
        os_log("starting")
        Thread.sleep(forTimeInterval: 2)
        value += 5
        os_log("done")
    }
    

    ... then it will likely say "15", not "20" because you'll see the notify for the workItem.perform() before the async call to the global queue is done. Now, you'd never use sleep in real apps, but I put it in to illustrate the timing issues.

    Bottom line, the notify on a DispatchWorkItem happens when the dispatch work item is first completed and it won't wait for the subsequent invocations of it. This code entails what is called a "race condition" between your notify block and the call you dispatched to that global queue and you're not assured which will run first.

  3. Personally, even setting aside the race conditions and the inherently non thread-safe behavior of mutating some variable from multiple threads, I'd advise against invoking the same DispatchWorkItem multiple times, at least in conjunction with notify on that work item.

  4. If you want to do a notification when everything is done, you should use a DispatchGroup, not a notify on the individual DispatchWorkItem.

Pulling this all together, you get something like:

import os.log

var value = 10
let lock = NSLock()   // a lock to synchronize our access to `value`

func notifyExperiment() {
    // rather than using `DispatchWorkItem`, a reference type, and invoking it multiple times,
    // let's just define some closure or function to run some task

    func performTask(message: String) {
        os_log("starting %@", message)
        Thread.sleep(forTimeInterval: 2)    // we wouldn't do this in production app, but lets do it here for pedagogic purposes, slowing it down enough so we can see what's going on
        lock.lock()
        value += 5
        lock.unlock()
        os_log("done %@", message)
    }

    // create a dispatch group to keep track of when these tasks are done

    let group = DispatchGroup()

    // let's enter the group so that we don't have race condition between dispatching tasks
    // to the queues and our notify process

    group.enter()

    // define what notification will be done when the task is done

    group.notify(queue: .main) {
        self.lock.lock()
        os_log("value = %d", self.value)
        self.lock.unlock()
    }

    // Let's run our task once on the global queue

    DispatchQueue.global(qos: .utility).async(group: group) {
        performTask(message: "from global queue")
    }

    // Let's run our task also on a custom queue

    let customQueue = DispatchQueue(label: "com.appcoda.delayqueue1", qos: .utility)
    customQueue.async(group: group) {
        performTask(message: "from custom queue")
    }

    // Now let's leave the group, resolving our `enter` at the top, allowing the `notify` block
    // to run iff (a) all `enter` calls are balanced with `leave` calls; and (b) once the `async(group:)`
    // calls are done.

    group.leave()
}