7

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")
    })

}
Lance Samaria
  • 17,576
  • 18
  • 108
  • 256

1 Answers1

16

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.

Rob
  • 415,655
  • 72
  • 787
  • 1,044
  • thanks for the help. I used WorkItems because I read they were GCD's equivalent of OperationQueues. The isCancelled is something I came across and that's why I used it. Even without using it the same problem occurs. What I want to do is if the user posts a pic, an acti Ind shows, the image data 1st goes to Storage and good then the url goes to the db. Since there are 2 steps anything can go wrong. If all good then alert the user. If it takes longer then x time period alert the user to try again. It was set for 15 secs and sometimes the alert would show for both. Print statements == Alerts – Lance Samaria Feb 18 '18 at 12:38
  • In my real app if it takes too long I dismiss the acti Ind and show a error label. If success I show an alert. This was just a shorter version that's why there's only 1 attempt. If the errorTask isn't cancelled in 0...1 secs that means in the callback in sendDataToFirebase() was never reached and self.errorTask?.cancel() and self.errorTask? = nil never ran. If they never run then errorTask should cancel the firebaseTask and if that's cancelled and set to nil then whatever is inside of it should be shut down also. That's why I did it that way. – Lance Samaria Feb 18 '18 at 12:45
  • Here's where I got isCancelled from. I just realized it's your answer. https://stackoverflow.com/a/38372384/4833705 But even when I used: firebaseTask = DispatchWorkItem{ [weak self] in self?.sendDataToFirebase() } *without* checking for isCancelled I still ran into the same problem. – Lance Samaria Feb 18 '18 at 12:50
  • @LanceSamaria - In that answer, we're doing some slow work and we're repeatedly checking whether it `isCancelled` throughout the process. So it makes sense there. (And, nonetheless, operation queues still offer a broader array of cancellation features, than GCD.) What doesn't make any sense is to check once and only once, and where the `DispatchWorkItem` starts. And unless Firebase provides some API to gracefully cancel the `updateChildValues`, something that you'd initiate when you cancel the task/operation, I'm not sure what the purpose of all of this cancellation stuff is, anyways. – Rob Feb 18 '18 at 15:23
  • Here’s something that will clarify this for me. updateChildValues runs inside the firebaseTask. If the firebaseTask is fired and updateChildValues is running, assuming it’s callback is never reached in xxx secs, if I cancel the firebaseTask and set it to nil, what happens to updateChildValues? Is it still running? If so what happens if it does or doesn’t connect? It sounds like your saying even though I cancel firebaseTask and set it to nil Im not cancelling updateChildValues because it’s already active – Lance Samaria Feb 18 '18 at 15:31
  • 1
    Yep, it’s still running. It will undoubtedly call its closure eventually, either reporting success or timing out according to its own criteria. Setting the state of the `DispatchWorkItem` has no bearing on that. (Sorry, I should have made that more explicit in my critique above, as it’s a fairly critical observation.) You can only cancel it if the API provides some cancellation method unique to the particular API in question. Some asynchronous API offer their own cancellation methods (e.g. `URLSession` or `CLGeocoder`) and other’s don’t. – Rob Feb 18 '18 at 16:10
  • Thank you very much for clarity. Now I get it. Would you mind putting that somewhere in your answer so I can accept it. Your other info was good but to me the main point is updateChildVales was still running even though firebaseTask was cancelled and set to nil. I’m not sure how many would realize that. Again thank you very much . Now I’m off to find a way to cancel updateChildValues – Lance Samaria Feb 18 '18 at 16:14
  • Updated answer accordingly. – Rob Feb 18 '18 at 16:22
  • Thanks for the clarification! – Lance Samaria Feb 18 '18 at 16:23
  • "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." Can you share real scenarios of where this is useful? e.g. uploading a 1Gb file may take a while, but that's not something I'd be having a for loop for. Maybe moving 1000 files? Where I can loop over each item? Anything else? – mfaani Oct 02 '19 at 14:56
  • Computationally intensive operations are the common example, such as image processing routines where you are iterating through the pixel buffer, pixel by pixel, doing some calculations. Or perhaps calculating a Mandelbrot set or calculating π or whatever. – Rob Oct 02 '19 at 15:28
  • Thanks. I'm doing none of those. Haven't heard of some of them :D I guess it's not much of a use to me. I should just use `Timer`. – mfaani Oct 02 '19 at 16:00
  • @Rob have you written any books ? i want to grab one. – BigFire Aug 18 '20 at 15:37