Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider marking runBlocking as @DelicateCoroutinesApi #4242

Open
joffrey-bion opened this issue Oct 4, 2024 · 4 comments
Open

Consider marking runBlocking as @DelicateCoroutinesApi #4242

joffrey-bion opened this issue Oct 4, 2024 · 4 comments

Comments

@joffrey-bion
Copy link
Contributor

joffrey-bion commented Oct 4, 2024

From the runBlocking documentation:

It is designed to bridge regular blocking code to libraries that are written in suspending style, to be used in main functions and in tests

Part of this is obsolete now. We have suspend fun main to get a suspending context right from the entrypoint. We have kotlinx-coroutines-test for tests.

This leaves the "briding" use case as the remaining valid case for runBlocking, but this is quite general/vague and runBlocking is still dangerous in many such cases. We have already seen a few nasty cases where runBlocking is synonym for trouble:

My intuition is that we should only use runBlocking when we have no other choice, and that there aren't many cases where runBlocking is the only choice (and is safe). In that sense, I believe it would be a net positive to mark runBlocking as @DelicateCoroutinesApi, with an updated doc to explain more details.

It would be worth documenting the cases that are safe for runBlocking, and the cases for which we know better alternatives. A bit like what was done for GlobalScope.

@qwwdfsad
Copy link
Member

That would be a nice idea if we were just to (re-)introduce run-blocking.

The problem is, it exists for quite a few years already, is really widespread (i.e. single DuckDuckGo app has 83 usages) and already leveraged in coursed, lectures, talks and books.

Marking it as a delicate API will likely create a serious disruption and a lot of immediate opt-ins (potentially module-wide), which will rather undermine the meaning and intent of @DelicateCoroutinesApi

@joffrey-bion
Copy link
Contributor Author

joffrey-bion commented Oct 31, 2024

Marking it as a delicate API will likely create a serious disruption and a lot of immediate opt-ins (potentially module-wide), which will rather undermine the meaning and intent of @DelicateCoroutinesApi

This is fair enough. Then here are my suggestions:

  • my points about docs still hold. I think the documentation of runBlocking should not mention obsolete use cases like main functions and tests, it should clearly warn about potential issues with unintentionally nested runBlocking calls (currently it's showing an obvious case instead of warning about non-obvious ones), and provide examples of right and wrong usages
  • instead of a (too harsh) opt-in annotation, we could provide an IDE inspection / soft warning to nudge users towards better approaches, like for readln. Even if there no obvious replacement (thus no quick-fix), it would still be useful.

@joffrey-bion
Copy link
Contributor Author

already leveraged in coursed, lectures, talks and books

To be honest I believe this argument is rather in favor of this change. Since runBlocking is so present in materials and gets so much attention, we need a strong opposing force to avoid it becoming more widespread. If wrong materials are written, they need to be corrected.

@kevincianfarini
Copy link
Contributor

kevincianfarini commented Nov 30, 2024

It's worth mentioning that as far as I understand currently, suspending main functions are not available on Kotlin/Native yet. I reported this issue in YouTrack a while back and it hasn't received any attention.

Also, if I understand correctly, runBlocking's event loop also implements delay so additional threads don't have to be utilized for delays. Suspending entry points don't and therefore would spin up the default delay dispatcher's threads to delay.

I agree that documentation about where it's safe to use run blocking would be nice. I imagine it would essentially amount to saying it's only safe to use when bridging blocking functions that are known to not have been called from a coroutine.

Finally, I agree that run blocking being widely used shouldn't be an impediment to marking it as a delicate API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants