Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Swift iOS -DispatchWorkItem is still running even though it's getting Cancelled and Set to Nil

I use GCD's DispatchWorkItem to keep track of my data that's being sent to firebase.

The first thing I do is declare 2 class properties of type DispatchWorkItem and then when I'm ready to send the data to firebase I initialize them with values.

The first property is named errorTask. When initialized it cancels the firebaseTask and sets it to nil then prints "errorTask fired". It has a DispatchAsync Timer that will call it in 0.0000000001 seconds if the errorTask isn't cancelled before then.

The second property is named firebaseTask. When initialized it contains a function that sends the data to firebase. If the firebase callback is successful then errorTask is cancelled and set to nil and then a print statement "firebase callback was reached" prints. I also check to see if the firebaseTask was cancelled.

The problem is the code inside the errorTask always runs before the firebaseTask callback is reached. The errorTask code cancels the firebaseTask and sets it to nil but for some reason the firebaseTask still runs. I can't figure out why?

The print statements support the fact that the errorTask runs first because "errorTask fired" always gets printed before "firebase callback was reached".

How come the firebaseTask isn't getting cancelled and set to nil even though the errorTask makes those things happen?

Inside my actual app what happens is if a user is sending some data to Firebase an activity indicator appears. Once the firebase callback is reached then the activity indicator is dismissed and an alert is shown to the user saying it was successful. However if the activity indicator doesn't have a timer on it and the callback is never reached then it will spin forever. The DispatchAsyc after has a timer set for 15 secs and if the callback isn't reached an error label would show. 9 out of 10 times it always works .

  1. send data to FB
  2. show activity indicator
  3. callback reached so cancel errorTask, set it to nil, and dismiss activity indicator
  4. show success alert.

But every once in while

  1. it would take longer then 15 secs
  2. firebaseTask is cancelled and set to nil, and the activity indicator would get dismissed
  3. the error label would show
  4. the success alert would still appear

The errorTask code block dismisses the actiInd, shows the errorLabel, and cancels the firebaseTask and sets it to nil. Once the firebaseTask is cancelled and set to nil I assumed everything inside of it would stop also because the callback was never reached. This may be the cause of my confusion. It seems as if even though the firebaseTask is cancelled and set to nil, someRef?.updateChildValues(... is somehow still running and I need to cancel that also.

My code:

var errorTask:DispatchWorkItem?
var firebaseTask:DispatchWorkItem?

@IBAction func buttonPush(_ sender: UIButton) {

    // 1. initialize the errorTask to cancel the firebaseTask and set it to nil
    errorTask = DispatchWorkItem{ [weak self] in
        self?.firebaseTask?.cancel()
        self?.firebaseTask = nil
        print("errorTask fired")
        // present alert that there is a problem
    }

    // 2. if the errorTask isn't cancelled in 0.0000000001 seconds then run the code inside of it
    DispatchQueue.main.asyncAfter(deadline: .now() + 0.0000000001, execute: self.errorTask!)

    // 3. initialize the firebaseTask with the function to send the data to firebase
    firebaseTask = DispatchWorkItem{ [weak self]  in

        // 4. Check to see the if firebaseTask was cancelled and if it wasn't then run the code
        if self?.firebaseTask?.isCancelled != true{
            self?.sendDataToFirebase()
        }

       // I also tried it WITHOUT using "if firebaseTask?.isCancelled... but the same thing happens
    }

    // 5. immediately perform the firebaseTask
    firebaseTask?.perform()
}

func sendDataToFirebase(){

    let someRef = Database.database().reference().child("someRef")

    someRef?.updateChildValues(myDict(), withCompletionBlock: {
        (error, ref) in

        // 6. if the callback to firebase is successful then cancel the errorTask and set it to nil
        self.errorTask?.cancel()
        self.errorTask? = nil

        print("firebase callback was reached")
    })

}
like image 803
Lance Samaria Avatar asked Feb 17 '18 17:02

Lance Samaria


1 Answers

This cancel routine is not doing what I suspect you think it is. When you cancel a DispatchWorkItem, it performs no preemptive cancellation. It certainly has no bearing on the updateChildValues call. All it does is perform a thread-safe setting of the isCancelled property, which if you were manually iterating through a loop, you could periodically check and exit prematurely if you see that the task was canceled.

As a result, the checking of isCancelled at the start of the task isn't terribly useful pattern, because if the task has not yet been created, there is nothing to cancel. Or if the task has been created and added to a queue, and canceled before the queue had a chance to start, it will obviously just be canceled but never started, you'll never get to your isCancelled test. And if the task has started, it's likely gotten past the isCancelled test before cancel was called.

Bottom line, attempts to time the cancel request so that they are received precisely after the task has started but before it has gotten to the isCancelled test is going to be an exercise in futility. You have a race that will be almost impossible to time perfectly. Besides, even if you did happen to time this perfectly, this merely demonstrates how ineffective this whole process is (only 1 in a million cancel requests will do what you intended).

Generally, if you had asynchronous task that you wanted to cancel, you'd wrap it in an asynchronous custom Operation subclass, and implement a cancel method that stops the underlying task. Operation queues simply offer more graceful patterns for canceling asynchronous tasks than dispatch queues do. But all of this presumes that the underlying asynchronous task offers a mechanism for canceling it and I don't know if Firebase even offers a meaningful mechanism to do that. I certainly haven't seen it contemplated in any of their examples. So all of this may be moot.

I'd suggest you step away from the specific code pattern in your question and describe what you are trying to accomplish. Let's not dwell on your particular attempted solution to your broader problem, but rather let's understand what the broader goal is, and then we can talk about how to tackle that.


As an aside, there are other technical issues in your example.

Specifically, I'm assuming you're running this on the main queue. So task.perform() runs it on the current queue immediately. But your DispatchQueue.main.asyncAfter(...) can only be run when whatever is running on the main queue is done. So, even though you specified a delay of 0.0000000001 seconds, it actually won't run until the main queue is available (namely, after your perform is done running on the main queue and you're well past the isCancelled test).

If you want to test this race between running the task and canceling the task, you need to perform the cancel on a different thread. For example, you could try:

weak var task: DispatchWorkItem?

let item = DispatchWorkItem {
    if (task?.isCancelled ?? true) {
        print("canceled")
    } else {
        print("not canceled in time")
    }
}

DispatchQueue.global().asyncAfter(deadline: .now() + 0.00001) {
    task?.cancel()
}

task = item
DispatchQueue.main.async {
    item.perform()
}

Now you can play with various delays and see the different behavior between a delay of 0.1 seconds and one of 0.0000000001 seconds. And you'll want to make sure the app has reached quiescence before you try this test (e.g. do it on a button press event, not in viewDidLoad).

But again, this will merely illustrate the futility of the whole exercise. You're going to have a really hard time catching the task between the time it started and before it checked the isCancelled property. If you really want to manifest the cancel logic in some repeatable manner, we're going to have to artificially make this happen:

weak var task: DispatchWorkItem?

let queue = DispatchQueue(label: "com.domain.app.queue") // create a queue for our test, as we never want to block the main thread

let semaphore = DispatchSemaphore(value: 0)

let item = DispatchWorkItem {
    // You'd never do this in a real app, but let's introduce a delay
    // long enough to catch the `cancel` between the time the task started.
    //
    // You could sleep for some interval, or we can introduce a semphore
    // to have it not proceed until we send a signal.

    print("starting")
    semaphore.wait() // wait for a signal before proceeding

    // now let's test if it is cancelled or not

    if (task?.isCancelled ?? true) {
        print("canceled")
    } else {
        print("not canceled in time")
    }
}

DispatchQueue.global().asyncAfter(deadline: .now() + 0.5) {
    task?.cancel()
    semaphore.signal()
}

task = item
queue.async {
    item.perform()
}

Now, you'd never do this, but it just illustrates that isCancelled does work.

Frankly, you'd never use isCancelled like this. You would generally use the isCancelled process if doing some long process where you can periodically check the isCancelled status and exit if it is true. But that's not the case in your situation.

The conclusion of all of this is that checking isCancelled at the start of a task is unlikely to ever achieve what you had hoped for.

like image 82
Rob Avatar answered Oct 08 '22 07:10

Rob