Appauth-android: performActionWithFreshTokens forces the callback to be ran in the UI/main thread

[camhart]

AuthState.performActionWithFreshTokens runs the AuthState.Action callback on the same thread as the calling thread when the token doesn't need refreshed, but when it does need refreshed it's always ran on the main/UI thread. I think this needs to get changed so that the AuthState.Action callback is always ran on either a background thread or the main/UI thread regardless of whether the token gets refreshed or not. And whichever is chosen should be clearly documented for other developers to read.

My guess is the programmers of AuthState.performActionWithFreshTokens assumed it would always be called from the main/UI thread... but there are situations where that's not the case.

[iainmcgin]

We hadn't documented the threading behavior of our callbacks and this is clearly an oversight, thanks for bringing it to our attention. I don't know if the callback should necessarily be forced to execute on the main thread or a background thread; this should likely be up to the developer. Perhaps this should be a parameter on the API itself, such as providing an Executor to indicate where the task should be run. I'll give it some thought for 0.5.0.

[camhart]

Another option would be to just leave it up to the developer to ensure the AuthState.performActionWithFreshTokens call is done within a background thread (and then be sure to update documentation to point out that thats needed). This leaves full control in the developers hands from what I can tell.

Plus in case the developer fails to read the documentation they'll get a nice exception telling them to change it.

[StevenEWright]

We should sync? https://github.com/openid/AppAuth-iOS/issues/46

[darrenjoconnor]

I vote for a thread stable approach. If the call was made from the UI thread then callback on the UI thread. If the call was made from background thread callback on background thread (never UI) regardless of the state of the refresh. It is the difference in thread treatment depending on if the refresh is performed or not that I tripped over.

[camhart]

As a temporary fix:
https://github.com/camhart/AppAuth-Android

[raehalme]

Any new thoughts on this? My vote is also on the thread stable approach.

[iainmcgin]

My intention is to provide two variants of the method, to which you can supply an Executor or a Handler. This will allow you to select the thread used with precision. The default behavior (where no executor or handler are provided) will remain the same.

[JessHolle]

I also am _very_ interested in a fix for this issue. Control over which thread/executor runs the background action and, more importantly, which runs the callback is key to some use cases. Insight into the timing of the 0.8.0 release would thus be appreciated.

[iainmcgin]

We generally don't commit to timelines for the releases, as we don't have any full-time maintainers (and my own time is stretched very thin across multiple efforts). However, I'll see what I can do about getting a fix for this in this week.