I'm reviewing some Alamofire sample Retrier code:
func should(_ manager: SessionManager, retry request: Request, with error: Error, completion: @escaping RequestRetryCompletion) {
lock.lock() ; defer { lock.unlock() }
if let response = request.task.response as? HTTPURLResponse, response.statusCode == 401 {
requestsToRetry.append(completion)
if !isRefreshing {
refreshTokens { [weak self] succeeded, accessToken, refreshToken in
guard let strongSelf = self else { return }
strongSelf.lock.lock() ; defer { strongSelf.lock.unlock() }
...
}
}
} else {
completion(false, 0.0)
}
}
I don't follow how you can have lock.lock() on the first line of the function and then also have that same line strongSelf.lock.lock() within the closure passed to refreshTokens.
If the first lock is not released until the end of the should method when the defer unlock is executed then how does the the second strongSelf.lock.lock() successfully execute while the first lock is held?
The trailing closure of
refreshTokens, where this second call tolock()/unlock()is called, runs asynchronously. This is because the closure is@escapingand is called from within aresponseJSONinside therefreshTokensroutine. So theshouldmethod will have performed its deferredunlockby the time the closure ofrefreshTokensis actually called.Having said that, this isn't the most elegant code that I've seen, where the utility of the lock is unclear and the risk of deadlocking is so dependent upon the implementation details of other routines. It looks like it's OK here, but I don't blame you for raising an eyebrow at it.