Prevent refresh race conditions #93
Conversation
| /// - parameter request: An unauthenticated NSURLRequest. | ||
| /// - parameter completion: A callback to invoke with the authenticated request. | ||
| public func authenticateRequest(request: NSURLRequest, completion: Result<NSURLRequest, NSError> -> ()) { | ||
| dispatch_sync(refreshQueue) { |
There was a problem hiding this comment.
Please note that if the completion block is called immediately, e.g. because there is no access token, and the completion block itself tries to authenticate another request, the program will deadlock. However, since the API is asynchronous anyways, you might want to use dispatch_async instead.
| @@ -39,6 +39,8 @@ public let HeimdallrErrorNotAuthorized = 2 | |||
| return accessToken != nil | |||
| } | |||
|
|
|||
| private var refreshQueue = dispatch_queue_create("de.rheinfabrik.Heimdallr.refeshQueue", DISPATCH_QUEUE_SERIAL) | |||
There was a problem hiding this comment.
We might think about using com.trivago.Heimdallr.refreshQueue here?
also: is it really a refreshQueue? Because all requests get queued here, right?
| @@ -208,6 +219,7 @@ public let HeimdallrErrorNotAuthorized = 2 | |||
| } | |||
| return .Failure(error) | |||
| })) | |||
| self.releaseRefreshQueue() | |||
There was a problem hiding this comment.
Would be a nice use case for a defer, right?
There was a problem hiding this comment.
I thought about that and since it can only be used in this completion block I left it out. But you are right, it's still a bit nicer. Fixed!
EDIT: after considering a bit more, it can be used everywhere else, it just looked a bit odd. I'll add it anyhow.
|
@marcelofabri do you have some comments on this one? |
|
Seems good to me! 👏 I didn't know about |
Description
Making multiple calls to
authenticateRequestwith an expired access token can lead to multiple requests for a new access token with the same refresh token. As a result, the user might be left with an invalid token.In order to prevent this, we decided to make calls to
authenticateRequestsequential. This is achieved by using a serial queue and limiting its resources via a semaphore.