Hi, ```fun OkhttpAuthenticator.authenticate() { ... synchronized(this) { runBlocking { retrofitS...

# coroutines

[ursus]

Hi,

fun OkhttpAuthenticator.authenticate() {
	...
	synchronized(this) {
		runBlocking { retrofitService.suspendingRefreshTokens() }<--- 
		...
	}
}

Could this get broken somehow? runBlocking seems to swap to caller thread context after the network call, so the mutex should be released

or is it not guaranteed? (Because I see certain bugs in production where the code just stops; and not sure if its runBlocning or retrofit related)

[bezrukov]

if OkhttpAuthenticator.authenticate() is invoked in okhttp's Interceptor/Authenticator and retrofitService uses the same okhttpclient it might be related to okhttp's Dispatcher (see maxRequests/maxRequestsPerHost).

[bezrukov]

if it is the case it's happening because all okhttp calls are invoked in okhttp.Dispatcher's executor with concurrency 5 (by default). Imagine you're started 5 queries and all of them require token refresh. When using runBlocking { suspendingRetrofitCall } it will schedule this call to the same executor and deadlock happens (because your 5 calls are waiting for token refresh, and token refresh call is waiting for these calls)

[ursus]

alright so the issue is that the retrofit call wants to switch threads, if I made it synchronous and on caller thread, it should be okay, right?

[ursus]

which would explain why it didnt manifest in rxjava, because there I didnt use the async parameter (which makes it use okhttp pool) and kept it on caller thread

[bezrukov]

if I made it synchronous and on caller thread, it should be okay, right?

yep should be ok, blocking calls bypass okhttp executor. Another option is to setup retrofit service for token refresh with dedicated okhttp dispatcher

[ursus]

you mean to drive Retrofit with a swapped out okhttp with a custom unbounded thread pool impl -- explicity for network calls made inside interceptors/authenticator?

[bezrukov]

no, just use yourOkHttpClient.newBuilder().dispatcher(Dispatcher()) when building your retrofitService service

[bezrukov]

in this case regular requests and requests for token refresh will be executed in different executors

[ursus]

yes thats what I meant

[ursus]

thanks for you help!

[ursus]

If I might add .. the issue is not necessarily the synchronization block right? even without it, if the threadpool is maxed out and I want a new (max+1) request within the authenticator, it will stall, right?

[bezrukov]

yep

[ursus]

Also, giving auth layer its own threadpool dispatcher, possibly wasteful no? doubling the threads, or is it negligible? I'd best have it run on caller thread (and still using the suspend api. in retrofit, to have cancelation), or coroutines dispatcher.io?

[bezrukov]

actually you do not double the threads because okhtttp creates executor with the following arguments:

ThreadPoolExecutor(0, Int.MAX_VALUE, 60, TimeUnit.SECONDS, ...

that means this pool will create threads lazily and will keep it only for 60 seconds (once they unused), so I would say it's negligible. Yep, best option would be to call it synchronously on caller thread

[ursus]

Do you know if its possible to have it on caller thread & keep suspend api? (i.e. the async parameter from former rxjava adapter?)

[bezrukov]

No, suspend adapters generated differently, so I think you need to just create sync adapter

[ursus]

Yea whats annoying the I have retrofit AuthApi, where now login is suspending, nad refreshTokens and logout is synchronous, not great design