https://kotlinlang.org logo
Title
d

David Khol

08/15/2021, 9:37 AM
Hi! 🙂 I am seeing a lot of developers ignoring structured concurrency by launching a new coroutine in a different scope, although they are already in a suspending function or have access to
CoroutineScope
. Real life example which I came across so many times can be a convenience method like this:
fun launchWithLoading(action: suspend () -> Unit) {
    scope.launch {
        try {
            loading = true
            action()
        } finally {
            loading = false
        }
    }
}
Nothing prevents the developer from calling it in a suspending context like:
suspend fun alreadySuspending() {
    launchWithLoading {
        // do stuff
    }
}
or like
fun first() {
    launchWithLoading {
        another()
    }
}

fun another() {
    launchWithLoading {
        // calculate something
    }
}
although rarely they intend to break the structured concurrency. I wonder if there is something like Android Lint but that is purely for Kotlin? Almost everything I've found on the internet points me in the direction of Android Lint. But since this unrelated to Android, I would like to solve it without coupling the solution to Android or Android Studio, so that it can be potentially used in IntelliJ IDEA as well.
d

Dominaezzz

08/15/2021, 9:41 AM
I don't think I'd say this is strictly breaking structured concurrency. I'd look at it like a convenience method. Once you inline the method, it looks a lot more clear why it's fine I think.
Although there should probably be some sort of semaphore around that loading boolean. Seems a bit thread unsafe.
d

David Khol

08/15/2021, 9:45 AM
I still find it unsafe since developers might do something like
fun first() {
    launchWithLoading {
        another()
        finishedEvents.value = Unit
    }
}

fun another() {
    launchWithLoading {
        // heavy calculations
    }
}
Now,
finishedEvents
will be triggered even though
another()
did not finish its work 😞
You are right to point out the
loading
being thread unsafe. I simplified it here but could be easily solved with
AtomicInteger
counting how many coroutines are currently "loading".
d

Dominaezzz

08/15/2021, 9:49 AM
The cost of abstraction I guess. With convenience comes ignorance and with ignorance comes carelessness.
launchWithLoading
should probably just be a suspend function there, so you're right.
d

David Khol

08/15/2021, 9:53 AM
I don't expect to make this work without additional hints to the lint/inspection/plugin since the intention can be unclear in many valid cases. I can imagine creating a new annotation, say
@RequiresNonSuspendingContext
that would work similarly to how
@Deprecated
or
@Nullable
annotations work. I am able to create an IDE plugin to visually highlight the incriminated piece of code as a warning/error, but that is still just highlighting. It would be better if it could be run as a part of Gradle build so that it could be used in CI as well.
d

Dominaezzz

08/15/2021, 10:10 AM
Could easily be a compiler plugin I think.
e

ephemient

08/15/2021, 5:20 PM
Android Lint can be applied to non-Android projects
t

Tomasz Krakowiak

08/16/2021, 9:47 AM
Recently posted in #feed - https://github.com/detekt/detekt/ I have no experience with it, just spotted possible relation.