Swift Code Running Twice When It Should Not

Question

I have an escaping function, which completes once a condition is met:

  private func xxxfastLoadLSecurityDescriptions(session: URLSession, mySymbols: [String]?, completion: @escaping(Bool) ->())    {
        
        var counter = mySymbols?.count ?? 0
        if counter == 0 { completion(false) }
        var doubleCount = 0
        // print("DESCRIPTION Starting Counter = \(counter)")
        
        for symbolIndex in 0..<(mySymbols?.count ?? 0) {
            guard let mySymbol = mySymbols?[symbolIndex] else { print("ERROR in fastLoadLSecurityDescriptions loop: No Symbol") ;  continue }
            guard let myGetDescriptionRequest = GenericDataRequest(dataToSend: [mySymbol], token: sessionToken, mgbRoute: MGBServerRoutes.retrieveSecurityDescriptionsRoute!)
            else { print("Error Getting Security Description Request for \(mySymbol)") ; return }
            
            mySessionSendMGBGenericRequest(session: session, request: myGetDescriptionRequest) { [weak self]  success, serverMessage, returnUNISecurityDescription in
                guard let self = self else { print("ERROR: self is nil") ; return }
                if returnUNISecurityDescription?.count == 0 { print("nil returnUniSecurityDescription for \(mySymbol)") }
                // print("DESCRIPTIONS COUNTER = \(counter)")
                counter -= 1
                var myDescription = UNISecurityDescription()
                if returnUNISecurityDescription != nil, returnUNISecurityDescription?.count != 0 { myDescription = returnUNISecurityDescription![0]  }
                if myDescription.name == nil || myDescription.name == "" { print("Error: No Name for \(String(describing: mySymbol))") }
                let myContainersIndices = self.myUNIList.singleContainer.indices.filter({ self.myUNIList.singleContainer[$0].ticker?.symbol == mySymbol })
                var myPathArray = [IndexPath]()
                for index in 0..<myContainersIndices.count {
                    self.myUNIList.singleContainer[myContainersIndices[index]].name = myDescription.name
                    self.myUNIList.singleContainer[myContainersIndices[index]].currencySymbol = myDescription.currencySymbol
                    self.myUNIList.singleContainer[myContainersIndices[index]].fillFundamentals() // --> Fills the outputs for sortdata
                    myPathArray.append(IndexPath(row: myContainersIndices[index], section: 0))
                }
                DispatchQueue.main.async {
                    self.filteredData = self.myUNIList.singleContainer
                    self.myCollection?.reloadItems(at: myPathArray)
                }
                if counter == 0     {   // THIS IS TRUE MORE THAN ONCE WHILE IT SHOULD NOT BE TRU MORE THAN ONCE
                    if doubleCount > 0 {    print("WHY!!!!")  }
                    doubleCount += 1
                    print("DESCRIPTIONS counter = \(counter) -> \(self.myUNIList.listName) - symbols: \(String(describing: mySymbols?.count)) \n==================================================\n")
                    DispatchQueue.main.async {   self.sortNormalTap("Load")  { _ in     self.displayAfterLoading()  } }
                    completion(true)
                    return
                }
            }
        }
    }

The condition to be met is counter == 0. Once this is met, the function completes and exits a DispatchGroup. The problem is that counter == 0 is true multiple times (with obvious crash in exiting the DispatchGroup). I really cannot understand why this condition is met more than once. The code is pretty linear and I cannot see what causes this. Any help is very appreciated. This is running me crazy.

`mySessionSendMGBGenericRequest` doesn't execute on the main thread, so when you change the counter within it's block, the other threads aren't going to know about it. You could make `counter` static so each block updates the same value, it's not a great way to do it as you should really be incrementing the counter in a some kind of callback or delegate method, but it should work. `static var counter = 0` and access the value with `Self.counter`. Probably should de-increment it within the `async` call as well. — clawesome, Nov 11 '20 at 18:15
@clawesome: The counter works fine. It has to be unique within the block. It is decremented for each return from mySessionSendMGBGenericRequest and it does linearly go to 0. That part works. What does not work is that the if counter == 0 is valued as true twice (or more), so I wonder what gets the execution there more than once. — Marco, Nov 11 '20 at 18:26
Seems like you‘re missing a return after the completion block when the counter is 0. The code after that if statement is still executing. I guess that‘s not what you want ‍♂️ — Goergisn, Nov 11 '20 at 20:32
Are you calling `xxxfastLoadLSecurityDescriptions` in a loop possibly? Also, are you expecting the `completion(true)` and the `return` after it to break your `for` loop on `mySymbols?.count`? — clawesome, Nov 11 '20 at 21:16

score 1 · Answer 1 · answered Nov 11 '20 at 22:05

You're code isn't thread safe, the counter in particular. I wrote an example using your same logic to show illustrate this. If you run it a few times, you will eventually hit the same condition in which your issue is happening.

override func viewDidLoad() {
    super.viewDidLoad()

    let mySymbols: [Int] = Array(0...100)

    for _ in 0..<100 {
        xxxfastLoadLSecurityDescriptions(session: URLSession.shared, mySymbols: mySymbols) { (success, counter, doubleCount) in
            print("Completed: \(success), Counter: \(counter), Double Count: \(doubleCount)")
        }
    }
}

private func xxxfastLoadLSecurityDescriptions(session: URLSession, mySymbols: [Int]?, completion: @escaping(Bool, Int, Int) ->())    {

    var counter = mySymbols?.count ?? 0

    if counter == 0 {
        return completion(false, -1, -1)
    }
    var doubleCount = 0

    for symbolIndex in 0..<(mySymbols?.count ?? 0) {
        guard let _ = mySymbols?[symbolIndex] else {
            print("Error")
            continue
        }

        DispatchQueue.global().asyncAfter(deadline: .now() + .milliseconds(Int.random(in: 50..<900))) {

            counter -= 1

            DispatchQueue.main.async {
                self.view.layoutIfNeeded()
            }

            if counter == 0 {
                if doubleCount > 0 {
                    // This will eventually print even though logically it shouldn't
                    print("*****Counter: \(counter), Double Count: \(doubleCount), Symbol Index: \(symbolIndex)")
                }
                doubleCount += 1
                completion(true, counter, doubleCount)
                return
            }
        }
    }
}

Output:

Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
*******************************************************************
*****   Counter: 0, Double Count: 1, Symbol Index: 15   
*******************************************************************
Completed: true, Counter: 0, Double Count: 2
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
*******************************************************************
*****   Counter: 0, Double Count: 1, Symbol Index: 26   
*******************************************************************
Completed: true, Counter: 0, Double Count: 2
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
Completed: true, Counter: 0, Double Count: 1
*******************************************************************
*****   Counter: 0, Double Count: 1, Symbol Index: 57   
*******************************************************************
Completed: true, Counter: 0, Double Count: 2
*******************************************************************
*****   Counter: 0, Double Count: 1, Symbol Index: 3    
*******************************************************************
Completed: true, Counter: 0, Double Count: 2

@goergisn: There is no code after the completion block. Adding a return does not change anything — Marco, Nov 11 '20 at 22:21
I see your comment. But I do not understand. I saw a lot of examples on how to count when all the required response from an HTTP call loop are returned with a logic like the one I implemented. How would you do it? — Marco, Nov 11 '20 at 22:27
The block shouldn't be responsible for de-incrementing the counter as it's out of it's scope. Between the time the block de-increments the counter and the time it checks it for 0, one of the other threads could have also de-incremented it. You should have a caller who has the counter and that caller should be de-incrementing the counter on completion of the callback. Can you link one of the examples you mentioned? — clawesome, Nov 11 '20 at 22:27
I think I understand now. Not sure hot to implement a thread safe way of keeping the count then. Your reply makes sense (apologies but I am not an async expert). Any help on how to do this is greatly appreciated. — Marco, Nov 11 '20 at 22:43

score 1 · Answer 2 · answered Nov 12 '20 at 08:57

At the end I solved this by using DispatchGroup as follows (simplified code):

private func xxxFastLoadLSecurityDescriptions(session: URLSession, mySymbols: [String]?, completion: @escaping(Bool) ->())    {
    
    let myGroup = DispatchGroup()
    
    for symbolIndex in 0..<(mySymbols?.count ?? 0) {
        
        myGroup.enter()
    
        mySessionSendMGBGenericRequest(...) { [weak self] returnValue in
            
            guard let self = self else { myGroup.leave() ; return }
         // do some stuff with returnValue
            
            myGroup.leave()
        }
    }
    
    myGroup.notify(queue: .main, execute: {
        
        completion(true)
    })
}

My question is: is it possible that I run into the same problem as before? For example, say I have a loop for 2 items. The firs item enters the group and say the before the second item enters the loop the async call returns a value and the first item exits the group. At this point, before the second item enters the group the .notify should be triggered and completion(true) exists the function before the second item is processed. Is this possible?

Swift Code Running Twice When It Should Not

2 Answers2