appmattus / certificatetransparency

Certificate transparency for Android and JVM
Apache License 2.0
142 stars 29 forks source link

DataSource reuseInflight job gets stuck in a cancelled state #97

Closed sudojhill closed 3 months ago

sudojhill commented 1 year ago

Maybe I am not fully understanding the code in DataSource . reuseInflight but if job gets cancelled before the line with launch gets called, then job is stuck in a cancelled state and subsequent calls to get() immediately fail. My app seemed to be getting into this situation and couldn't recover.

For reference:

override suspend fun get(): Value? {
    return coroutineScope {
        job ?: async { this@DataSource.get() }.apply {
            job = this

            launch {
                this@apply.join()
                job = null
            }
        }
    }.await()
}

Adding job?.invokeOnCompletion { job = null } seems to fix the issue I was experiencing:

override suspend fun get(): Value? {
    return coroutineScope {
        job ?: async { this@DataSource.get() }.apply {
            job = this
            job?.invokeOnCompletion { job = null }

            launch {
                this@apply.join()
                job = null
            }
        }
    }.await()
}
mattmook commented 3 months ago

Yes, this looks like a sensible solution and probably should have always been written like this! I've created a unit test that shows this solution works (and breaks on the original code)