New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(AWSMobileClient): Fixing a crash when getTokens is called while tokenOperations is being iterated. #4563
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to test these scenarios in AWSMobileClient and Amplify v1.
- Successful fetching of tokens
- Successful fetching of tokens after it is expired, AWSMobileClient should refresh the tokens automatically if refresh token is valid
- Receive token expired notification in case refresh token is invalid
@@ -88,6 +88,8 @@ final public class AWSMobileClient: _AWSMobileClient { | |||
var userPassword: String? = nil | |||
|
|||
var tokenOperations:NSHashTable<FetchUserPoolTokensOperation> = NSHashTable.weakObjects() | |||
let tokenOperationsQueue: DispatchQueue = DispatchQueue(label:"AWSMobileClient.tokenOperationsQueue", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is similarly named queue tokenFetchOperationQueue
and can be confusing. Should we rename this queueToAccessTokenOperations
or some other name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, it becomes confusing.
So I instead decided to encapsulate this behaviour inside a new WeakHashTable
class
…he tokenOperations are being iterated.
e1dd019
to
b0fd669
Compare
…okenOperations is being iterated. (aws-amplify#4563)
Issue #, if available:
#4486
Description of changes:
Calling
AWSMobileClient.getTokens(_:)
at the same time a user is signing out/in could result in a crash, since thetokenOperations
is Iterated when the auth state changes.This happens because
NSHashTable
's operations are not thread safe, so I'm creating atokenOperationsQueue
queue that we can use to read/write from the collection. I'm setting its.concurrent
attribute so reads can be done concurrently, but setting the.barrier
flag when actually inserting values.I've also wrapped this implementation in a new class called
WeakHashTable
, so it reduces the change of misuse in the future (e.g. if we add another read/write from the collection in a different place without using the queue)Check points:
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.