search

telenordigital / connect-android-sdk

Android SDK for CONNECT ID
https://telenordigital.github.io/id-docs.telenordigital.com/
Other
16 stars 14 forks source link

ConnectTokens in ConnectIdService is not threadsafe #138

Open govkannan opened 5 years ago

govkannan commented 5 years ago

ConnectTokens in ConnectIdService seems not threadsafe and getting different reference of ConnectTokens when used in multi-threaded call.

In MyTelenor android app, ConnectSDK.getValidAccessToken() is called and on success response to this call, a different thread is started to get the token. And old value is returned in the second call.

ConnectTokens should be made tread safe.

govkannan commented 5 years ago

ConnectTokens are updated in callback response, which would be called on thread. https://github.com/telenordigital/connect-android-sdk/blob/master/connect/src/com/telenor/connect/id/ConnectIdService.java#L90

It requires synchronize whereever ConnectTokens is accessed and updated

jorunfa commented 5 years ago

line 90 only covers the initial tokens, line 268 is for the update method's writing of the currentTokens.

Ref from email:

ConnectSdk.getValidAccessToken() -> a new token is updated and callback method onSuccess. And onSuccess, we start a new thread which queries the ConnectToken attribute (for eg, expirationTime), The ConnectIdService returns old token value.

updateTokens only calls callback.success (L269) after writing the variable on L268. It seems strange to me that new value has not been written at this time, but I guess it could be the case.

What method are you calling that is using ConnectIdService (and making it return the old value)?

govkannan commented 5 years ago

Yes, in L268, the token is updated which I assume it is done in other thread.

What method are you calling that is using ConnectIdService (and making it return the old value)? getAccessTokenExpirationTime() which returned old time and after this, calling updateTokens() sends old token in request (as you can see in the mail response from Ulf)

jorunfa commented 5 years ago

I'll see if I'm able to recreate the problem. If you have a snippet already available that would be helpful.

govkannan commented 5 years ago

It is very random and tricky to reproduce. To simulate the issue, you need to run two threads, started in different cpu processor and updating value from one thread and reading it from other. As explained in this example http://tutorials.jenkov.com/java-concurrency/volatile.html