How to cancel a thread in swift

Question

I am facing a problem where a class instance is not being deallocated, so the app gets stuck. The class which is causing the problem is

import Foundation

class ResponseTimeoutQueue{

    private let mMsgOut: [UInt8]

    private let mResponseMsgId: Int

    private let mTimeoutMilliSec: UInt32

    private let mNumRetries: Int

    private var timeoutWorkItem: DispatchWorkItem?

    init(responseMsgId: Int, msgOut: [UInt8], timeoutMilliSec: UInt32, numRetries: Int) {
        mResponseMsgId = responseMsgId
        mMsgOut = msgOut
        mTimeoutMilliSec = timeoutMilliSec
        mNumRetries = numRetries
    }

    func run() {
    
        // create a work item with the custom code
        timeoutWorkItem = DispatchWorkItem {
                 // Insert your code here
           
               var retryNum: Int = 0
               var doRetry: Bool = true
               while (doRetry) {
                   Thread.sleep(forTimeInterval: TimeInterval(self.mTimeoutMilliSec))
                   // If we are here then it means the last send did not receive a response before
                   // timing out.  Write with no timeout or num retries so we don't spawn another
                   // ResponseTimeoutQueue.
     

                   retryNum += 1
                   if (retryNum <= self.mNumRetries) {
                       SessionController.sharedController.invokeWriteData(responseMsgId: self.mResponseMsgId, bytes: self.mMsgOut)
                   } else {
                       doRetry = false
                   }
               }
               // Notify the handler.
               NotificationCenter.default.post(name: .timeOutMessage, object: -1)
           }

          //Create dispatch group
           let dispatchGroup = DispatchGroup()

          // execute the workItem with dispatchGroup
           DispatchQueue.global().async(group: dispatchGroup, execute: timeoutWorkItem!)

          //Handle code after the completion of global queue
           dispatchGroup.notify(queue: DispatchQueue.global()) {
               }

            timeoutWorkItem?.cancel()
    }

    func interrupt() {
        timeoutWorkItem?.cancel()
        timeoutWorkItem = nil
    }
}

The calling of run() function occurs from

func writeNotify(responseMsgId: Int, buffer: [UInt8], timeoutMilliSec: UInt32, numRetries: Int) {
    if (timeoutMilliSec > 0) {
        mResponseMonitorThreadMap[responseMsgId]?.interrupt()
        let timeoutThread: ResponseTimeoutQueue =
        ResponseTimeoutQueue(responseMsgId: responseMsgId, msgOut: buffer,
                             timeoutMilliSec: timeoutMilliSec, numRetries: numRetries)
        
        timeoutThread.run()
        mResponseMonitorThreadMap[responseMsgId] = timeoutThread
    }
}

It is basically we are requesting data from radar and when we get the data we call

func cancelResponseTimeout(responseMsgId: Int) {
    mResponseMonitorThreadMap[responseMsgId]?.interrupt()
    if mResponseMonitorThreadMap.keys.contains(responseMsgId){
        mResponseMonitorThreadMap[responseMsgId] = nil
    }
}

But after some minutes of communicating with the radar the thread count of responseTimeoutQueue number is 65 or more. I commented the writeToNotify() function, then the application has no problem. I will attach the memory hierarchy screenshot. I have tried giving timeoutThread = nil after we call timeoutThread.run() but the class instance number is not changing. How to resolve the issue? Thanks in advance.

Answer 1

I can see a number of issues here. The first and probably most important is at this line:

Thread.sleep(forTimeInterval: TimeInterval(self.mTimeoutMilliSec))

A TimeInterval is an alias for Double and it represents a number of seconds. If you want to sleep for say 500 milliseconds, you need TimeInterval(500)/1000 or TimeInterval(0.5).

I'm also not sure what the point of all the calls to cancel() is. The one in run() is a race condition since it might get invoked before the work item begins executing. The other one does nothing because cancel() will only cancel items that have not yet started executing.

You do have a strong reference cycle, but I think it would be broken if you set timeoutWorkItem to nil in the dispatch group completion handler.

Talking of the dispatch group: you only have one item, so you don't really need it. You can put a completion handler directly on the work item and set the property to nil there.

Edit

OK, just had another look. The biggest problem isn't the mega long timeouts, it's the fact that cancel() does not do what you think. This is the documentation:

Cancellation causes future attempts to execute the work item to return immediately. Cancellation does not affect the execution of a work item that has already begun.

[my italics]

I think you'd be much better off setting an NSTimer or a DispatchSourceTimer for the time out and cancelling it if you get a response and then it's event block can do the retry.