What is the best practice to refresh OAuth access tokens using a refresh token in OkHttp? I have the following setup, but it's failing in case of parallel calls. The

synchronized(lock)

doesn't seem to prevent the second call to try to refresh the token as well.

val okHttpClient = OkHttpClient.Builder()
    .addInterceptor(AccessTokenInterceptor(oAuthRepository))
    .authenticator(RefreshTokenAuthenticator(oAuthRepository))
    .build()

val lock = Any()

private class AccessTokenInterceptor(
    private val oAuthRepository: OAuthRepository
) : Interceptor {

    override fun intercept(chain: Chain): Response {
        synchronized(lock) {
            val token = oAuthRepository.accessToken

            val request = chain.request().newBuilder().apply {
                if (token != null) header("Authorization", "Bearer $token")
            }.build()
            return chain.proceed(request)
        }
    }
}

private class RefreshTokenAuthenticator(
    private val oAuthRepository: OAuthRepository
) : Authenticator {

    override fun authenticate(route: Route?, response: Response): Request? {
        if (response.responseCount >= 2) return null

        synchronized(lock) {
            return runBlocking {
                oAuthRepository.refreshTokens().map {
                    oAuthRepository.accessToken
                }
            }.fold(
                ifLeft = { null },
                ifRight = {
                    it?.let { token ->
                        response.request.newBuilder()
                            .header("Authorization", "Bearer $token")
                            .build()
                    }
                }
            )
        }
    }
}

Just a wild guess, but are the two refreshes happening for the same call? Might be the same thread and reentrant?

I'm quite sure they happen for two subsequent calls. But now, after reading a few examples on the web, I think I know what's missing. The

synchronized

block for the second (and subsequent) calls is not skipped - it's just delayed. That means every next call tries (unnecessarily) to refresh the token (which has just been refreshed). So after entering the synchronized block I should check if the token has changed, and in case it's the new token, skip the refresh and just use it to authenticate. This means I have to add:

override fun authenticate(route: Route?, response: Response): Request? {
    val oldToken = oAuthRepository.accessToken

    synchronized(lock) {
        val newToken = oAuthRepository.accessToken
        // if the newToken is different, it means that it has been refreshed in the mean time by another call
        // so we can just use it to authenticate and skip the refresh
        if (newToken != oldToken) return response.request.newBuilder()
            .header("Authorization", "Bearer $newToken")
            .build()
        // Otherwise this is the first call trying to refresh a token - so just refresh it
        // Previous refresh flow
     }
}

do not hold a lock while calling

return chain.proceed(request)

unless you want to avoid all paralellism

Thank you, I was also thinking the lock in the

AccessTokenInterceptor

was actually unnecessary/harmful.

This is what I use and it seems to work without issues. I've stress tested it with charles proxy and send a bunch of 401s and stuff and it seems to work perfectly. Maybe not the best code... but it works. https://gist.github.com/ColtonIdle/3da8930a486ae5d88ae014a93539a0ac

Thanks. I now check if the access token is still the same after entering the

synchronized

block. If it changed, that means it was already refreshed in another thread and I just skip the refresh and use the new token.